{"thread":{"id":"37991","subject":"[PATCH v3 02/14] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update","startedAt":"2014-11-18T01:35:36Z","lastAt":"2014-11-27T05:34:45Z","messageCount":31,"participants":["Stefan Beller","Michael Haggerty","Ronnie Sahlberg","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":3,"patchTotal":14},"messages":[{"id":"252023","messageId":"1416274550-2827-1-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":null,"subject":"[PATCH v3 00/14] ref-transactions-reflog","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:36Z","receivedAt":"2014-11-18T01:35:36Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi,\n\nThe following patch series updates the reflog handling to use transactions.\nThis patch series has previously been sent to the list[1].\n\nThis series converts the reflog handling and builtin/reflog.c to use\na transaction for both the ref as well as the reflog updates.\nAs a side effect of this it simplifies the reflog marshalling code so that we\nonly have one place where we marshall the entry.\nIt also means that we can remove several functions from the public api\ntowards the end of the series since we no longer need those functions.\n\nThis series can also be found at github[2] or at googlesource[3].\nFeel free to review, where it suits you best.\n\n\nVersion 3:\n * Go over the commit messages and reword them slightly where appropriate.\n   (only cosmetics, like missing/double words, spelling, clarify)\n * As Ronnie announced to change employers soon, he'll have only limited \n   time to work on git in the near future. As this is a rather large patch\n   series, he is handing this work over to me. That's why I'm sending the\n   patches this time.\n\nThanks,\nStefan\n\n[1] http://www.spinics.net/lists/git/msg241186.html\n[2] https://github.com/stefanbeller/git/tree/ref-transactions-reflog\n[3] https://code-review.googlesource.com/#/q/topic:ref-transaction-reflog\n\nRonnie Sahlberg (14):\n  refs.c: make ref_transaction_create a wrapper for\n    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 function to append a reflog entry to a fd\n  refs.c: add a new update_type field to ref_update\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: Remove unlock_ref/close_ref/commit_ref from the refs api\n  refs.c: remove lock_any_ref_for_update\n  refs.c: allow deleting refs with a broken sha1\n\n branch.c                    |  13 +-\n builtin/branch.c            |   5 +-\n builtin/checkout.c          |   8 +-\n builtin/commit.c            |  10 +-\n builtin/fetch.c             |  12 +-\n builtin/receive-pack.c      |  13 +-\n builtin/reflog.c            |  85 ++++------\n builtin/replace.c           |  10 +-\n builtin/tag.c               |  10 +-\n builtin/update-ref.c        |  26 +--\n cache.h                     |   7 +\n fast-import.c               |  22 +--\n refs.c                      | 403 +++++++++++++++++++++++++++++---------------\n refs.h                      |  87 +++++-----\n sequencer.c                 |  12 +-\n t/t1402-check-ref-format.sh |   8 +\n walker.c                    |  10 +-\n 17 files changed, 440 insertions(+), 301 deletions(-)\n\n-- \n2.2.0.rc2.5.gf7b9fb2\n"},{"id":"252036","messageId":"1416274550-2827-2-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 01/14] refs.c: make ref_transaction_create a wrapper for ref_transaction_update","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:37Z","receivedAt":"2014-11-18T01:35:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nThe ref_transaction_update function can already be used to create refs by\npassing null_sha1 as the old_sha1 parameter. Simplify by replacing\ntransaction_create with a thin wrapper.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n refs.c | 27 ++-------------------------\n 1 file changed, 2 insertions(+), 25 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 5ff457e..005eb18 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3623,31 +3623,8 @@ 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-\tassert(err);\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-\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n-\t\tstrbuf_addf(err, \"refusing to create ref with bad name %s\",\n-\t\t\t    refname);\n-\t\treturn -1;\n-\t}\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,\n-- \n2.2.0.rc2.5.gf7b9fb2\n"},{"id":"252022","messageId":"1416274550-2827-3-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 02/14] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:38Z","receivedAt":"2014-11-18T01:35:38Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n refs.c | 22 ++--------------------\n refs.h |  2 +-\n 2 files changed, 3 insertions(+), 21 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 005eb18..05cb299 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3633,26 +3633,8 @@ 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-\tassert(err);\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+\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 2bc3556..7d675b7 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -283,7 +283,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, 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-- \n2.2.0.rc2.5.gf7b9fb2\n"},{"id":"252033","messageId":"1416274550-2827-4-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 03/14] refs.c: rename the transaction functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:39Z","receivedAt":"2014-11-18T01:35:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nRename the transaction functions. Remove the leading ref_ from the\nnames and append _ref to the names for functions that create/delete/\nupdate sha1 refs.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n branch.c               | 13 +++++----\n builtin/commit.c       | 10 +++----\n builtin/fetch.c        | 12 ++++----\n builtin/receive-pack.c | 13 ++++-----\n builtin/replace.c      | 10 +++----\n builtin/tag.c          | 10 +++----\n builtin/update-ref.c   | 26 ++++++++---------\n fast-import.c          | 22 +++++++-------\n refs.c                 | 78 +++++++++++++++++++++++++-------------------------\n refs.h                 | 36 +++++++++++------------\n sequencer.c            | 12 ++++----\n walker.c               | 10 +++----\n 12 files changed, 126 insertions(+), 126 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 4bab55a..c8462de 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -279,16 +279,17 @@ void create_branch(const char *head,\n \t\tlog_all_ref_updates = 1;\n \n \tif (!dont_change_ref) {\n-\t\tstruct ref_transaction *transaction;\n+\t\tstruct 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_ref(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\tstrbuf_release(&err);\n \t}\n \ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e108c53..f50b7df 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1673,7 +1673,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstruct stat statbuf;\n \tstruct commit *current_head = NULL;\n \tstruct commit_extra_header *extra = NULL;\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n@@ -1804,17 +1804,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    transaction_update_ref(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_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 7b84d35..0be0b09 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -404,7 +404,7 @@ static int s_update_ref(const char *action,\n {\n \tchar msg[1024];\n \tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \tint ret, df_conflict = 0;\n \n@@ -414,23 +414,23 @@ 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    transaction_update_ref(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) {\n \t\tdf_conflict = (ret == TRANSACTION_NAME_CONFLICT);\n \t\tgoto fail;\n \t}\n \n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \tstrbuf_release(&err);\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 32fc540..397abc9 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -838,26 +838,25 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t}\n \telse {\n \t\tstruct strbuf err = STRBUF_INIT;\n-\t\tstruct ref_transaction *transaction;\n+\t\tstruct transaction *transaction;\n \n \t\tif (shallow_update && si->shallow_ref[cmd->index] &&\n \t\t    update_shallow_ref(cmd, si))\n \t\t\treturn \"shallow error\";\n \n-\t\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    transaction_update_ref(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\tref_transaction_free(transaction);\n-\n+\t\t    transaction_commit(transaction, &err)) {\n+\t\t\ttransaction_free(transaction);\n \t\t\trp_error(\"%s\", err.buf);\n \t\t\tstrbuf_release(&err);\n \t\t\treturn \"failed to update ref\";\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 85d39b5..5a7ab1f 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -155,7 +155,7 @@ static int replace_object_sha1(const char *object_ref,\n \tunsigned char prev[20];\n \tenum object_type obj_type, repl_type;\n \tchar ref[PATH_MAX];\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n \tobj_type = sha1_object_info(object, NULL);\n@@ -169,14 +169,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    transaction_update_ref(transaction, ref, repl, prev,\n \t\t\t\t   0, 1, NULL, &err) ||\n-\t    ref_transaction_commit(transaction, &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 e633f4e..5f3554b 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -583,7 +583,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tconst char *msgfile = NULL, *keyid = NULL;\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n \tstruct commit_list *with_commit = NULL;\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n@@ -730,13 +730,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    transaction_update_ref(transaction, ref.buf, object, prev,\n \t\t\t\t   0, 1, NULL, &err) ||\n-\t    ref_transaction_commit(transaction, &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 6c9be05..af08dd9 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -175,7 +175,7 @@ static int parse_next_sha1(struct strbuf *input, const char **next,\n  * depending on how line_termination is set.\n  */\n \n-static const char *parse_cmd_update(struct ref_transaction *transaction,\n+static const char *parse_cmd_update(struct transaction *transaction,\n \t\t\t\t    struct strbuf *input, const char *next)\n {\n \tstruct strbuf err = STRBUF_INIT;\n@@ -198,7 +198,7 @@ static const char *parse_cmd_update(struct ref_transaction *transaction,\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+\tif (transaction_update_ref(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@@ -209,7 +209,7 @@ static const char *parse_cmd_update(struct ref_transaction *transaction,\n \treturn next;\n }\n \n-static const char *parse_cmd_create(struct ref_transaction *transaction,\n+static const char *parse_cmd_create(struct transaction *transaction,\n \t\t\t\t    struct strbuf *input, const char *next)\n {\n \tstruct strbuf err = STRBUF_INIT;\n@@ -229,7 +229,7 @@ static const char *parse_cmd_create(struct ref_transaction *transaction,\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+\tif (transaction_create_ref(transaction, refname, new_sha1,\n \t\t\t\t   update_flags, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n@@ -240,7 +240,7 @@ static const char *parse_cmd_create(struct ref_transaction *transaction,\n \treturn next;\n }\n \n-static const char *parse_cmd_delete(struct ref_transaction *transaction,\n+static const char *parse_cmd_delete(struct transaction *transaction,\n \t\t\t\t    struct strbuf *input, const char *next)\n {\n \tstruct strbuf err = STRBUF_INIT;\n@@ -264,7 +264,7 @@ static const char *parse_cmd_delete(struct ref_transaction *transaction,\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+\tif (transaction_delete_ref(transaction, refname, old_sha1,\n \t\t\t\t   update_flags, have_old, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n@@ -275,7 +275,7 @@ static const char *parse_cmd_delete(struct ref_transaction *transaction,\n \treturn next;\n }\n \n-static const char *parse_cmd_verify(struct ref_transaction *transaction,\n+static const char *parse_cmd_verify(struct transaction *transaction,\n \t\t\t\t    struct strbuf *input, const char *next)\n {\n \tstruct strbuf err = STRBUF_INIT;\n@@ -300,7 +300,7 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction,\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+\tif (transaction_update_ref(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@@ -320,7 +320,7 @@ static const char *parse_cmd_option(struct strbuf *input, const char *next)\n \treturn next + 8;\n }\n \n-static void update_refs_stdin(struct ref_transaction *transaction)\n+static void update_refs_stdin(struct transaction *transaction)\n {\n \tstruct strbuf input = STRBUF_INIT;\n \tconst char *next;\n@@ -376,9 +376,9 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \n \tif (read_stdin) {\n \t\tstruct strbuf err = STRBUF_INIT;\n-\t\tstruct ref_transaction *transaction;\n+\t\tstruct transaction *transaction;\n \n-\t\ttransaction = ref_transaction_begin(&err);\n+\t\ttransaction = transaction_begin(&err);\n \t\tif (!transaction)\n \t\t\tdie(\"%s\", err.buf);\n \t\tif (delete || no_deref || argc > 0)\n@@ -386,9 +386,9 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin(transaction);\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\tstrbuf_release(&err);\n \t\treturn 0;\n \t}\ndiff --git a/fast-import.c b/fast-import.c\nindex d0bd285..152d944 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1687,7 +1687,7 @@ found_entry:\n static int update_branch(struct branch *b)\n {\n \tstatic const char *msg = \"fast-import\";\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tunsigned char old_sha1[20];\n \tstruct strbuf err = STRBUF_INIT;\n \n@@ -1713,17 +1713,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    transaction_update_ref(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_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 \tstrbuf_release(&err);\n \treturn 0;\n }\n@@ -1745,9 +1745,9 @@ static void dump_tags(void)\n \tstruct tag *t;\n \tstruct strbuf ref_name = STRBUF_INIT;\n \tstruct strbuf err = STRBUF_INIT;\n-\tstruct ref_transaction *transaction;\n+\tstruct 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@@ -1756,17 +1756,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\tif (transaction_update_ref(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 05cb299..2044d8f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -26,7 +26,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_ref when a loose ref is being\n  * pruned.\n  */\n #define REF_ISPRUNING\t0x0100\n@@ -2533,23 +2533,23 @@ static void try_remove_empty_parents(char *name)\n /* make sure nobody touched the ref, and unlink */\n static void prune_ref(struct ref_to_prune *r)\n {\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n \tif (check_refname_format(r->name, 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    transaction_delete_ref(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_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 \tstrbuf_release(&err);\n \ttry_remove_empty_parents(r->name);\n }\n@@ -2711,20 +2711,20 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\n \n int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n {\n-\tstruct ref_transaction *transaction;\n+\tstruct 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    transaction_delete_ref(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_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 \tstrbuf_release(&err);\n \treturn 0;\n }\n@@ -3531,9 +3531,9 @@ struct ref_update {\n  *         an active transaction or if there is a failure while building\n  *         the transaction thus rendering it failed/inactive.\n  */\n-enum ref_transaction_state {\n-\tREF_TRANSACTION_OPEN   = 0,\n-\tREF_TRANSACTION_CLOSED = 1\n+enum transaction_state {\n+\tTRANSACTION_OPEN   = 0,\n+\tTRANSACTION_CLOSED = 1\n };\n \n /*\n@@ -3541,21 +3541,21 @@ enum ref_transaction_state {\n  * consist of checks and updates to multiple references, carried out\n  * as atomically as possible.  This structure is opaque to callers.\n  */\n-struct ref_transaction {\n+struct transaction {\n \tstruct ref_update **updates;\n \tsize_t alloc;\n \tsize_t nr;\n-\tenum ref_transaction_state state;\n+\tenum transaction_state state;\n };\n \n-struct ref_transaction *ref_transaction_begin(struct strbuf *err)\n+struct transaction *transaction_begin(struct strbuf *err)\n {\n \tassert(err);\n \n-\treturn xcalloc(1, sizeof(struct ref_transaction));\n+\treturn xcalloc(1, sizeof(struct transaction));\n }\n \n-void ref_transaction_free(struct ref_transaction *transaction)\n+void transaction_free(struct transaction *transaction)\n {\n \tint i;\n \n@@ -3570,7 +3570,7 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \tfree(transaction);\n }\n \n-static struct ref_update *add_update(struct ref_transaction *transaction,\n+static struct ref_update *add_update(struct transaction *transaction,\n \t\t\t\t     const char *refname)\n {\n \tsize_t len = strlen(refname);\n@@ -3582,7 +3582,7 @@ 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_ref(struct 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@@ -3593,7 +3593,7 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \n \tassert(err);\n \n-\tif (transaction->state != REF_TRANSACTION_OPEN)\n+\tif (transaction->state != TRANSACTION_OPEN)\n \t\tdie(\"BUG: update called for transaction that is not open\");\n \n \tif (have_old && !old_sha1)\n@@ -3617,23 +3617,23 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \treturn 0;\n }\n \n-int ref_transaction_create(struct ref_transaction *transaction,\n+int transaction_create_ref(struct 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-\treturn ref_transaction_update(transaction, refname, new_sha1,\n+\treturn transaction_update_ref(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+int transaction_delete_ref(struct 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-\treturn ref_transaction_update(transaction, refname, null_sha1,\n+\treturn transaction_update_ref(transaction, refname, null_sha1,\n \t\t\t\t      old_sha1, flags, have_old, msg, err);\n }\n \n@@ -3641,17 +3641,17 @@ int update_ref(const char *action, const char *refname,\n \t       const unsigned char *sha1, const unsigned char *oldval,\n \t       int flags, enum action_on_err onerr)\n {\n-\tstruct ref_transaction *t;\n+\tstruct 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    transaction_update_ref(t, refname, sha1, oldval, flags,\n \t\t\t\t   !!oldval, action, &err) ||\n-\t    ref_transaction_commit(t, &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);\n@@ -3666,7 +3666,7 @@ int update_ref(const char *action, const char *refname,\n \t\treturn 1;\n \t}\n \tstrbuf_release(&err);\n-\tref_transaction_free(t);\n+\ttransaction_free(t);\n \treturn 0;\n }\n \n@@ -3694,8 +3694,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 transaction *transaction,\n+\t\t       struct strbuf *err)\n {\n \tint ret = 0, delnum = 0, i;\n \tconst char **delnames;\n@@ -3704,11 +3704,11 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \n \tassert(err);\n \n-\tif (transaction->state != REF_TRANSACTION_OPEN)\n+\tif (transaction->state != TRANSACTION_OPEN)\n \t\tdie(\"BUG: commit called for transaction that is not open\");\n \n \tif (!n) {\n-\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\t\ttransaction->state = TRANSACTION_CLOSED;\n \t\treturn 0;\n \t}\n \n@@ -3787,7 +3787,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \tclear_loose_ref_cache(&ref_cache);\n \n cleanup:\n-\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\ttransaction->state = TRANSACTION_CLOSED;\n \n \tfor (i = 0; i < n; i++)\n \t\tif (updates[i]->lock)\ndiff --git a/refs.h b/refs.h\nindex 7d675b7..556adfd 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -11,22 +11,22 @@ struct ref_lock {\n };\n \n /*\n- * A ref_transaction represents a collection of ref updates\n+ * A transaction represents a collection of ref updates\n  * that should succeed or fail together.\n  *\n  * Calling sequence\n  * ----------------\n- * - Allocate and initialize a `struct ref_transaction` by calling\n- *   `ref_transaction_begin()`.\n+ * - Allocate and initialize a `struct transaction` by calling\n+ *   `transaction_begin()`.\n  *\n  * - List intended ref updates by calling functions like\n- *   `ref_transaction_update()` and `ref_transaction_create()`.\n+ *   `transaction_update_ref()` and `transaction_create_ref()`.\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@@ -42,7 +42,7 @@ struct ref_lock {\n  * The message is appended to err without first clearing err.\n  * err will not be '\\n' terminated.\n  */\n-struct ref_transaction;\n+struct transaction;\n \n /*\n  * Bit values set in the flags argument passed to each_ref_fn():\n@@ -181,8 +181,8 @@ extern int is_branch(const char *refname);\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_ref(),\n+ * transaction_create_ref(), etc.\n  * REF_NODEREF: act on the ref directly, instead of dereferencing\n  *              symbolic references.\n  * REF_DELETING: tolerate broken refs\n@@ -269,13 +269,13 @@ 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 transaction *transaction_begin(struct strbuf *err);\n \n /*\n  * The following functions add a reference check or update to a\n- * ref_transaction.  In all of them, refname is the name of the\n+ * transaction.  In all of them, refname is the name of the\n  * reference to be affected.  The functions make internal copies of\n  * refname and msg, so the caller retains ownership of these parameters.\n  * flags can be REF_NODEREF; it is passed to update_ref_lock().\n@@ -291,7 +291,7 @@ 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+int transaction_update_ref(struct 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@@ -307,7 +307,7 @@ 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+int transaction_create_ref(struct 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@@ -321,7 +321,7 @@ 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+int transaction_delete_ref(struct 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@@ -337,13 +337,13 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n #define TRANSACTION_NAME_CONFLICT -1\n /* All other errors. */\n #define TRANSACTION_GENERIC_ERROR -2\n-int ref_transaction_commit(struct ref_transaction *transaction,\n-\t\t\t   struct strbuf *err);\n+int transaction_commit(struct 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 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 a03d4fa..f888005 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -238,7 +238,7 @@ static int error_dirty_index(struct replay_opts *opts)\n static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \t\t\tint unborn, struct replay_opts *opts)\n {\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf err = STRBUF_INIT;\n \n@@ -248,13 +248,13 @@ 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\",\n+\t    transaction_update_ref(transaction, \"HEAD\",\n \t\t\t\t   to, unborn ? null_sha1 : from,\n \t\t\t\t   0, 1, sb.buf, &err) ||\n-\t    ref_transaction_commit(transaction, &err)) {\n-\t\tref_transaction_free(transaction);\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@@ -263,7 +263,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \n \tstrbuf_release(&sb);\n \tstrbuf_release(&err);\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \treturn 0;\n }\n \ndiff --git a/walker.c b/walker.c\nindex f149371..f1d5e9b 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -253,7 +253,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n {\n \tstruct strbuf refname = STRBUF_INIT;\n \tstruct strbuf err = STRBUF_INIT;\n-\tstruct ref_transaction *transaction = NULL;\n+\tstruct transaction *transaction = NULL;\n \tunsigned char *sha1 = xmalloc(targets * 20);\n \tchar *msg = NULL;\n \tint i, ret = -1;\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 done;\n@@ -298,7 +298,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \t\t\tcontinue;\n \t\tstrbuf_reset(&refname);\n \t\tstrbuf_addf(&refname, \"refs/%s\", write_ref[i]);\n-\t\tif (ref_transaction_update(transaction, refname.buf,\n+\t\tif (transaction_update_ref(transaction, refname.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@@ -306,7 +306,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \t\t\tgoto done;\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 \t\tgoto done;\n \t}\n@@ -314,7 +314,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \tret = 0;\n \n done:\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \tfree(msg);\n \tfree(sha1);\n \tstrbuf_release(&err);\n-- \n2.2.0.rc2.5.gf7b9fb2\n"},{"id":"252025","messageId":"1416274550-2827-5-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 04/14] refs.c: add a function to append a reflog entry to a fd","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:40Z","receivedAt":"2014-11-18T01:35:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nBreak out the code to create the string and writing it to the file\ndescriptor from log_ref_write and add it into a dedicated function\nlog_ref_write_fd. For now this is only used from log_ref_write,\nbut later on we will call this function from reflog transactions too,\nwhich means that we will end up with only a single place,\nwhere we write a reflog entry to a file instead of the current two\nplaces (log_ref_write and builtin/reflog.c).\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n refs.c | 48 ++++++++++++++++++++++++++++++------------------\n 1 file changed, 30 insertions(+), 18 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 2044d8f..f0f0d23 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2990,15 +2990,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@@ -3010,19 +3032,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.2.0.rc2.5.gf7b9fb2\n"},{"id":"252024","messageId":"1416274550-2827-6-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 05/14] refs.c: add a new update_type field to ref_update","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:41Z","receivedAt":"2014-11-18T01:35:41Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nAdd 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 <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n refs.c | 27 +++++++++++++++++++++++----\n 1 file changed, 23 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex f0f0d23..84e086f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3516,6 +3516,10 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n \treturn retval;\n }\n \n+enum transaction_update_type {\n+\tUPDATE_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@@ -3523,6 +3527,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@@ -3583,12 +3588,14 @@ void transaction_free(struct transaction *transaction)\n }\n \n static struct ref_update *add_update(struct 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@@ -3618,7 +3625,7 @@ int transaction_update_ref(struct transaction *transaction,\n \t\treturn -1;\n \t}\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@@ -3696,13 +3703,17 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \n \tassert(err);\n \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\tstrbuf_addf(err,\n \t\t\t\t    \"Multiple updates for ref '%s' not allowed.\",\n \t\t\t\t    updates[i]->refname);\n \t\t\treturn 1;\n \t\t}\n+\t}\n \treturn 0;\n }\n \n@@ -3734,13 +3745,17 @@ int transaction_commit(struct 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 \t\tint flags = update->flags;\n \n+\t\tif (update->update_type != UPDATE_SHA1)\n+\t\t\tcontinue;\n+\n \t\tif (is_null_sha1(update->new_sha1))\n \t\t\tflags |= REF_DELETING;\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@@ -3762,6 +3777,8 @@ int transaction_commit(struct 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\tif (write_ref_sha1(update->lock, update->new_sha1,\n \t\t\t\t\t   update->msg)) {\n@@ -3779,6 +3796,8 @@ int transaction_commit(struct 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 = TRANSACTION_GENERIC_ERROR;\n-- \n2.2.0.rc2.5.gf7b9fb2\n"},{"id":"252030","messageId":"1416274550-2827-7-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 06/14] refs.c: add a transaction function to append a reflog entry","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:42Z","receivedAt":"2014-11-18T01:35:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nDefine 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 <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n refs.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--\n refs.h |  12 ++++++++\n 2 files changed, 112 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 84e086f..9a46e1c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3517,7 +3517,8 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n }\n \n enum transaction_update_type {\n-\tUPDATE_SHA1 = 0\n+\tUPDATE_SHA1 = 0,\n+\tUPDATE_LOG = 1\n };\n \n /**\n@@ -3535,6 +3536,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@@ -3581,6 +3588,7 @@ void transaction_free(struct 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@@ -3601,6 +3609,41 @@ static struct ref_update *add_update(struct transaction *transaction,\n \treturn update;\n }\n \n+int transaction_update_reflog(struct 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 != TRANSACTION_OPEN)\n+\t\tdie(\"BUG: update_reflog called for transaction that is not 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_ref(struct transaction *transaction,\n \t\t\t   const char *refname,\n \t\t\t   const unsigned char *new_sha1,\n@@ -3773,7 +3816,28 @@ int transaction_commit(struct 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\tstrbuf_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@@ -3809,6 +3873,40 @@ int transaction_commit(struct 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 = TRANSACTION_GENERIC_ERROR;\n \t\tgoto cleanup;\ndiff --git a/refs.h b/refs.h\nindex 556adfd..8220d18 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -328,6 +328,18 @@ int transaction_delete_ref(struct 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 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.\n  *\n-- \n2.2.0.rc2.5.gf7b9fb2\n"},{"id":"252027","messageId":"1416274550-2827-8-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 07/14] refs.c: add a flag to allow reflog updates to truncate the log","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:43Z","receivedAt":"2014-11-18T01:35:43Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nAdd a flag that allows us to truncate the reflog before we write the\nupdate.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\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 9a46e1c..d21ecb9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3873,7 +3873,12 @@ int transaction_commit(struct 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@@ -3881,7 +3886,15 @@ int transaction_commit(struct 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 8220d18..5075073 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -328,7 +328,15 @@ int transaction_delete_ref(struct 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 transaction *transaction,\n \t\t\t      const char *refname,\n-- \n2.2.0.rc2.5.gf7b9fb2\n"},{"id":"252032","messageId":"1416274550-2827-9-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 08/14] refs.c: only write reflog update if msg is non-NULL","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:44Z","receivedAt":"2014-11-18T01:35:44Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nWhen 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\nThis change only affects whether or not a reflog entry should be generated\nand written. If msg==NULL then no such entry will be written.\n\nOrthogonal to this we have a boolean flag REFLOG_TRUNCATE which is used to\ntell the transaction system to \"truncate the reflog and thus discard all\nprevious users\".\n\nAt the current time the only place where we use msg==NULL is also the\nplace, where we use REFLOG_TRUNCATE. Even though these two settings are\ncurrently only ever used together it still makes sense to have them through\ntwo separate knobs.\n\nThis allows future consumers of this API that may want to do things\ndifferently. For example someone can do:\n  msg=\"Reflog truncated by Bob because ...\" + REFLOG_TRUNCATE\nand have it truncate the log and have it start fresh with an initial message\nthat explains the log was truncated. This API allows that.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\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 d21ecb9..3572977 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3895,8 +3895,9 @@ int transaction_commit(struct 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 5075073..bf96b36 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -337,6 +337,7 @@ int transaction_delete_ref(struct 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 transaction *transaction,\n \t\t\t      const char *refname,\n-- \n2.2.0.rc2.5.gf7b9fb2\n"},{"id":"252026","messageId":"1416274550-2827-10-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 09/14] refs.c: allow multiple reflog updates during a single transaction","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:45Z","receivedAt":"2014-11-18T01:35:45Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nAllow 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-something...\n   transaction_reflog_update(t, \"foo\", 0, <message>);\ntransaction_commit(t)\n\nwhere we first truncate the reflog and then build the new content one line\nat a time.\n\nWhile this technically looks like O(n2) behavior it is not that bad.\nWe only do this loop for transactions that cover a single ref during\nreflog expire. This means that the linear search inside\ntransaction_update_reflog() will find the match on the very first entry\nthus making it O(1) and not O(n) or our usecases. Thus the whole expire\nbecomes O(n) instead of O(n2). If in the future we start doing this for many\nrefs in one single transaction we might want to optimize this.\nBut there is no need to complexify the code and optimize for future usecases\nthat might never materialize at this stage.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n refs.c | 48 +++++++++++++++++++++++++++++++++++++++---------\n 1 file changed, 39 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 3572977..0fb4196 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -31,6 +31,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@@ -3521,7 +3527,7 @@ enum transaction_update_type {\n \tUPDATE_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@@ -3531,7 +3537,9 @@ 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;  /* The flags to transaction_update_ref[log] are defined\n+\t\t     * in refs.h\n+\t\t     */\n \tint have_old; /* 1 if old_sha1 is valid, 0 otherwise */\n \tstruct ref_lock *lock;\n \tint type;\n@@ -3539,8 +3547,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@@ -3619,11 +3628,26 @@ int transaction_update_reflog(struct transaction *transaction,\n \t\t\t      struct strbuf *err)\n {\n \tstruct ref_update *update;\n+\tint i;\n \n \tif (transaction->state != TRANSACTION_OPEN)\n \t\tdie(\"BUG: update_reflog called for transaction that is not 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@@ -3639,7 +3663,6 @@ int transaction_update_reflog(struct transaction *transaction,\n \t}\n \tif (msg)\n \t\tupdate->msg = xstrdup(msg);\n-\tupdate->flags = flags;\n \n \treturn 0;\n }\n@@ -3822,10 +3845,15 @@ int transaction_commit(struct 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_NO_DEREF);\n \t\tif (update->reflog_fd < 0) {\n \t\t\tconst char *str = \"Cannot lock reflog for '%s'. %s\";\n \n@@ -3891,7 +3919,7 @@ int transaction_commit(struct 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@@ -3901,7 +3929,7 @@ int transaction_commit(struct 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@@ -3912,9 +3940,11 @@ int transaction_commit(struct 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.2.0.rc2.5.gf7b9fb2\n"},{"id":"252034","messageId":"1416274550-2827-11-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 10/14] reflog.c: use a reflog transaction when writing during expire","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:46Z","receivedAt":"2014-11-18T01:35:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nUse a transaction for all updates during expire_reflog.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/reflog.c | 85 ++++++++++++++++++++++++--------------------------------\n refs.c           |  4 +--\n refs.h           |  2 +-\n 3 files changed, 40 insertions(+), 51 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 2d85d26..6bb7454 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 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,20 @@ 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\t\tlock->lk->filename.buf);\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_ref(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\tgoto cleanup;\n \t\t}\n+\t\tif (transaction_commit(cb.t, &err))\n+\t\t\tstatus |= error(\"%s\", err.buf);\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 0fb4196..1571fa5 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3622,7 +3622,7 @@ int transaction_update_reflog(struct 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@@ -3903,7 +3903,7 @@ int transaction_commit(struct 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 bf96b36..9f70b89 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -343,7 +343,7 @@ int transaction_update_reflog(struct 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.2.0.rc2.5.gf7b9fb2\n"},{"id":"252035","messageId":"1416274550-2827-12-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 11/14] refs.c: rename log_ref_setup to create_reflog","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:47Z","receivedAt":"2014-11-18T01:35:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nlog_ref_setup is used to do several semi-related things:\n* Sometimes it will create a new reflog including missing parent\n  directories and cleaning up any conflicting stale directories\n  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: In checkout.c, where\nit is always used to create a reflog and in refs.c log_ref_write,\nwhere it is used to create a reflog sometimes, and sometimes just\nto fill in the filename.\n\nRename log_ref_setup to create_reflog and change it to only take the\nrefname as an argument to make its signature similar to delete_reflog\nand reflog_exists. Change create_reflog to ignore log_all_ref_updates\nand \"unconditionally\" create the reflog when called. Since checkout.c\nalways wants to create a reflog we can call create_reflog directly and\navoid the temp-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 <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\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 5410dac..8550b6d 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -587,19 +587,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 1571fa5..11cf26b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2947,16 +2947,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@@ -3025,16 +3025,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 9f70b89..17e3a3c 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -207,11 +207,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 int flags,\n \t\t       unsigned long at_time, int cnt,\n@@ -221,6 +216,9 @@ extern int read_ref_at(const char *refname, unsigned int flags,\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.2.0.rc2.5.gf7b9fb2\n"},{"id":"252031","messageId":"1416274550-2827-13-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 12/14] refs.c: Remove unlock_ref/close_ref/commit_ref from the refs api","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:48Z","receivedAt":"2014-11-18T01:35:48Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nunlock|close|commit_ref can be made static since there are no more external\ncallers.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\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 11cf26b..e49ae11 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2096,6 +2096,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@@ -2894,7 +2904,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@@ -2902,7 +2912,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@@ -2910,16 +2920,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 17e3a3c..025e2cb 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -198,15 +198,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 int flags,\n \t\t       unsigned long at_time, int cnt,\n-- \n2.2.0.rc2.5.gf7b9fb2\n"},{"id":"252029","messageId":"1416274550-2827-14-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 13/14] refs.c: remove lock_any_ref_for_update","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:49Z","receivedAt":"2014-11-18T01:35:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nNo one is using this function so we can delete it.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n refs.c | 7 -------\n refs.h | 9 +--------\n 2 files changed, 1 insertion(+), 15 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex e49ae11..b318888 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2352,13 +2352,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, NULL, flags, type_p);\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 025e2cb..721e21f 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -181,8 +181,7 @@ extern int is_branch(const char *refname);\n extern int peel_ref(const char *refname, unsigned char *sha1);\n \n /*\n- * Flags controlling lock_any_ref_for_update(), transaction_update_ref(),\n- * transaction_create_ref(), etc.\n+ * Flags controlling transaction_update_ref(), transaction_create_ref(), etc.\n  * REF_NODEREF: act on the ref directly, instead of dereferencing\n  *              symbolic references.\n  * REF_DELETING: tolerate broken refs\n@@ -191,12 +190,6 @@ extern int peel_ref(const char *refname, unsigned char *sha1);\n  */\n #define REF_NODEREF\t0x01\n #define REF_DELETING\t0x02\n-/*\n- * This function sets errno to something 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 int flags,\n-- \n2.2.0.rc2.5.gf7b9fb2\n"},{"id":"252028","messageId":"1416274550-2827-15-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"[PATCH v3 14/14] refs.c: allow deleting refs with a broken sha1","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T01:35:50Z","receivedAt":"2014-11-18T01:35:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nAdd back support to make it possible to delete refs that have a broken\nsha1.\n\nAdd new internal flags REF_ALLOW_BROKEN and RESOLVE_REF_ALLOW_BAD_SHA1\nto pass intent from branch.c that we are willing to allow\nresolve_ref_unsafe and lock_ref_sha1_basic to allow broken refs.\nSince these refs can not actually be resolved to a sha1, they instead resolve\nto null_sha1 when these flags are used.\n\nFor example, the ref:\n\n   echo \"Broken ref\" > .git/refs/heads/foo-broken-1\n\ncan now be deleted using git branch -d foo-broken-1\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/branch.c            | 5 +++--\n cache.h                     | 7 +++++++\n refs.c                      | 6 ++++++\n refs.h                      | 6 ++++--\n t/t1402-check-ref-format.sh | 8 ++++++++\n 5 files changed, 28 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 3b79c50..04f57d4 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -238,7 +238,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\ttarget = resolve_ref_unsafe(name,\n \t\t\t\t\t    RESOLVE_REF_READING\n \t\t\t\t\t    | RESOLVE_REF_NO_RECURSE\n-\t\t\t\t\t    | RESOLVE_REF_ALLOW_BAD_NAME,\n+\t\t\t\t\t    | RESOLVE_REF_ALLOW_BAD_NAME\n+\t\t\t\t\t    | RESOLVE_REF_ALLOW_BAD_SHA1,\n \t\t\t\t\t    sha1, &flags);\n \t\tif (!target) {\n \t\t\terror(remote_branch\n@@ -255,7 +256,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (delete_ref(name, sha1, REF_NODEREF)) {\n+\t\tif (delete_ref(name, sha1, REF_NODEREF|REF_ALLOW_BROKEN)) {\n \t\t\terror(remote_branch\n \t\t\t      ? _(\"Error deleting remote branch '%s'\")\n \t\t\t      : _(\"Error deleting branch '%s'\"),\ndiff --git a/cache.h b/cache.h\nindex 99ed096..61e61af 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1000,10 +1000,17 @@ extern int read_ref(const char *refname, unsigned char *sha1);\n  * resolved. The function returns NULL for such ref names.\n  * Caps and underscores refers to the special refs, such as HEAD,\n  * FETCH_HEAD and friends, that all live outside of the refs/ directory.\n+ *\n+ * RESOLVE_REF_ALLOW_BAD_SHA1 when this flag is set and the ref contains\n+ * an invalid sha1, resolve_ref_unsafe will clear the sha1 argument,\n+ * set the REF_ISBROKEN flag and return the refname.\n+ * This allows using resolve_ref_unsafe to check for existence of such\n+ * broken refs.\n  */\n #define RESOLVE_REF_READING 0x01\n #define RESOLVE_REF_NO_RECURSE 0x02\n #define RESOLVE_REF_ALLOW_BAD_NAME 0x04\n+#define RESOLVE_REF_ALLOW_BAD_SHA1 0x08\n extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);\n extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);\n \ndiff --git a/refs.c b/refs.c\nindex b318888..d75af6c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1584,6 +1584,10 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned\n \t\t\t    (buffer[40] != '\\0' && !isspace(buffer[40]))) {\n \t\t\t\tif (flags)\n \t\t\t\t\t*flags |= REF_ISBROKEN;\n+\t\t\t\tif (resolve_flags & RESOLVE_REF_ALLOW_BAD_SHA1) {\n+\t\t\t\t\thashclr(sha1);\n+\t\t\t\t\treturn refname;\n+\t\t\t\t}\n \t\t\t\terrno = EINVAL;\n \t\t\t\treturn NULL;\n \t\t\t}\n@@ -2265,6 +2269,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t\tif (flags & REF_NODEREF)\n \t\t\tresolve_flags |= RESOLVE_REF_NO_RECURSE;\n \t}\n+\tif (flags & REF_ALLOW_BROKEN)\n+\t\tresolve_flags |= RESOLVE_REF_ALLOW_BAD_SHA1;\n \n \trefname = resolve_ref_unsafe(refname, resolve_flags,\n \t\t\t\t     lock->old_sha1, &type);\ndiff --git a/refs.h b/refs.h\nindex 721e21f..2e97f4f 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -185,11 +185,13 @@ extern int peel_ref(const char *refname, unsigned char *sha1);\n  * REF_NODEREF: act on the ref directly, instead of dereferencing\n  *              symbolic references.\n  * REF_DELETING: tolerate broken refs\n+ * REF_ALLOW_BROKEN: allow locking refs that are broken.\n  *\n  * Flags >= 0x100 are reserved for internal use.\n  */\n-#define REF_NODEREF\t0x01\n-#define REF_DELETING\t0x02\n+#define REF_NODEREF\t\t0x01\n+#define REF_DELETING\t\t0x02\n+#define REF_ALLOW_BROKEN\t0x04\n \n /** Reads log for the value of ref during at_time. **/\n extern int read_ref_at(const char *refname, unsigned int flags,\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex e5dc62e..a0aef69 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -197,4 +197,12 @@ invalid_ref_normalized 'heads///foo.lock'\n invalid_ref_normalized 'foo.lock/bar'\n invalid_ref_normalized 'foo.lock///bar'\n \n+test_expect_success 'git branch -d can delete ref with broken sha1' '\n+\techo \"012brokensha1\" > .git/refs/heads/brokensha1 &&\n+\ttest_when_finished \"rm -f .git/refs/heads/brokensha1\" &&\n+\tgit branch -d brokensha1 &&\n+\tgit branch >output &&\n+\t! grep -e \"brokensha1\" output\n+'\n+\n test_done\n-- \n2.2.0.rc2.5.gf7b9fb2\n"},{"id":"252080","messageId":"546B2CE0.6020208@alum.mit.edu","threadId":"37991","inReplyTo":"1416274550-2827-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH v3 00/14] ref-transactions-reflog","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-18T11:26:24Z","receivedAt":"2014-11-18T11:26:24Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/18/2014 02:35 AM, Stefan Beller wrote:\n> The following patch series updates the reflog handling to use transactions.\n> This patch series has previously been sent to the list[1].\n> [...]\n\nI was reviewing this patch series (I left some comments in Gerrit about\nthe first few patches) when I realized that I'm having trouble\nunderstanding the big picture of where you want to go with this. I have\nthe feeling that the operations that you are implementing are at too low\na level of abstraction.\n\nWhat are the elementary write operations that are needed for a reflog?\nOff the top of my head,\n\n1. Add a reflog entry when a reference is updated in a transaction.\n2. Rename a reflog file when the corresponding reference is renamed.\n3. Delete the reflog when the corresponding reference is deleted [1].\n4. Configure a reference to be reflogged.\n5. Configure a reference to not be reflogged anymore and delete any\n   existing reflog.\n6. Selectively expire old reflog entries, e.g., based on their age.\n\nHave I forgotten any?\n\nThe first three should be side-effects of the corresponding reference\nupdates. Aside from the fact that renames are not yet done within a\ntransaction, I think this is already the case.\n\nNumber 4, I think, currently only happens in conjunction with adding a\nline to the reflog. So it could be implemented, say, as a\nFORCE_CREATE_REFLOG flag on a ref_update within a transaction.\n\nNumber 5 is not very interesting, I think. For example, it could be a\nseparate API function, disconnected from any transactions.\n\nNumber 6 is more interesting, and from my quick reading, it looks like a\nlot of the work of this patch series is to allow number 6 to be\nimplemented in builtin/reflog.c:expire_reflog(). But it seems to me that\nyou are building API calls at the wrong level of abstraction. Expiring a\nreflog should be a single API call to the refs API, and ultimately it\nshould be left up to the refs backend to decide how to implement it. For\na filesystem-based backend, it would do what it does now. But (for\nexample) a SQL-based backend might implement this as a single SELECT\nstatement.\n\nI also don't have the feeling that reflog expiration has to be done\nwithin a ref_transaction. For example, is there ever a reason to combine\nexpiration with other reference updates in a single atomic transaction?\nI think not.\n\nSo it seems to me that it would be more practical to have a separate API\nfunction that is called to expire selected entries from a reflog [2],\nunconnected with any transaction.\n\nI am not nearly as steeped in this code as you and Ronnie, and it could\nbe that I'm forgetting lots of details that make your design preferable.\nBut other reviewers are probably in the same boat. So I think it would\nbe really helpful if you would provide a high-level description of the\nAPI that you are proposing, and some discussion of its design and\ntradeoffs. A big part of this description could go straight into a file\nDocumentation/technical/api-ref-transactions.txt, which will be a great\n(and necessary) resource soon anyway.\n\nMichael\n\n[1] Though hopefully there will be future reference backends that don't\nhave to discard reflogs when a reference is deleted, so let's not bake\nthis behavior too fundamentally into the API.\n\n[2] ...and/or possibly one to expire reflogs for multiple references, if\nperformance would benefit significantly.\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"252099","messageId":"CAL=YDWn1x9TMGOWrmT5KMpQ_iBR0AQ5Ej1yr1pBb4==k0-vchw@mail.gmail.com","threadId":"37991","inReplyTo":"546B2CE0.6020208@alum.mit.edu","subject":"Re: [PATCH v3 00/14] ref-transactions-reflog","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-11-18T18:36:44Z","receivedAt":"2014-11-18T18:36:44Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"On Tue, Nov 18, 2014 at 3:26 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> On 11/18/2014 02:35 AM, Stefan Beller wrote:\n>> The following patch series updates the reflog handling to use transactions.\n>> This patch series has previously been sent to the list[1].\n>> [...]\n>\n> I was reviewing this patch series (I left some comments in Gerrit about\n> the first few patches) when I realized that I'm having trouble\n> understanding the big picture of where you want to go with this. I have\n> the feeling that the operations that you are implementing are at too low\n> a level of abstraction.\n>\n> What are the elementary write operations that are needed for a reflog?\n> Off the top of my head,\n>\n> 1. Add a reflog entry when a reference is updated in a transaction.\n> 2. Rename a reflog file when the corresponding reference is renamed.\n> 3. Delete the reflog when the corresponding reference is deleted [1].\n> 4. Configure a reference to be reflogged.\n> 5. Configure a reference to not be reflogged anymore and delete any\n>    existing reflog.\n> 6. Selectively expire old reflog entries, e.g., based on their age.\n>\n> Have I forgotten any?\n>\n> The first three should be side-effects of the corresponding reference\n> updates. Aside from the fact that renames are not yet done within a\n> transaction, I think this is already the case.\n>\n> Number 4, I think, currently only happens in conjunction with adding a\n> line to the reflog. So it could be implemented, say, as a\n> FORCE_CREATE_REFLOG flag on a ref_update within a transaction.\n>\n> Number 5 is not very interesting, I think. For example, it could be a\n> separate API function, disconnected from any transactions.\n>\n> Number 6 is more interesting, and from my quick reading, it looks like a\n> lot of the work of this patch series is to allow number 6 to be\n> implemented in builtin/reflog.c:expire_reflog(). But it seems to me that\n> you are building API calls at the wrong level of abstraction. Expiring a\n> reflog should be a single API call to the refs API, and ultimately it\n> should be left up to the refs backend to decide how to implement it. For\n> a filesystem-based backend, it would do what it does now. But (for\n> example) a SQL-based backend might implement this as a single SELECT\n> statement.\n\nI agree in principle. But things are more difficult since\nexpire_reflog() has very complex semantics.\nTo keep things simple for the reviews at this stage the logic is the\nsame as the original code:\n  loop over all entries:\n     use very complex conditionals to decide which entries to keep/remove\n     optionally modify the sha1 values for the records we keep\n     write records we keep back to the file one record at a time\n\nSo that as far as possible, we keep the same rules and behavior but we\nuse a different API for the actual\n\"write entry to new reflog\".\n\n\nWe could wrap this inside a new specific transaction_expire_reflog()\nfunction so that other types of backends, for example an SQL backend,\ncould optimize, but I think that should be in a separate later patch\nbecause expire_reflog is almost impossibly complex.\nIt will not be a simple SELECT unfortunately.\n\nThe current expire logic is something like :\n  1, expire all entries older than timestamp\n  2, optionally, also expire all entries that refer to unreachable\nobjects using a different timestamp\n      This involves actually reading the objects that the sha1 points\nto and parsing them!\n  3, optionally, if the sha1 objects can not be referenced, they are\nnot commit objects or if they don't exist, then expire them too.\n      This also involves reading the objects behind the sha1.\n  4, optionally, delete reflog entry #foo\n  5, optionally, if any log entries were discarded due to 2,3,4 then\nwe might also re-write and modify some of the reflog entries we keep.\nor any combination thereof\n\n  (6, if --dry-run is specified, just print what we would have expired)\n\n\n2 and 3 requires that we need to read the objects for the entry\n4 allows us to delete a specific entry\n5 means that even for entries we keep we will need to mutate them.\n\n\n\n\n\n\n>\n> I also don't have the feeling that reflog expiration has to be done\n> within a ref_transaction. For example, is there ever a reason to combine\n> expiration with other reference updates in a single atomic transaction?\n\n--updateref\nIn expire_reflog() we not only prune the reflog. When --updateref is\nused we update the actual ref itself.\nI think we want to have both the ref update and also the reflog update\nboth be part of a single atomic transaction.\n\n\n> I think not.\n>\n> So it seems to me that it would be more practical to have a separate API\n> function that is called to expire selected entries from a reflog [2],\n> unconnected with any transaction.\n\nI think it makes the API cleaner if we have a\n'you can only update a ref/reflog/<other things added in the future>/\nfrom within a transaction.'\n\nSince we need to do reflog changes within a transaction for the expire\nreflog case as well as the rename ref case\nI think it makes sense to enforce that reflog changes must be done\nwithin a transaction to just make it consistent.\n\n\n\n>\n> I am not nearly as steeped in this code as you and Ronnie, and it could\n> be that I'm forgetting lots of details that make your design preferable.\n> But other reviewers are probably in the same boat. So I think it would\n> be really helpful if you would provide a high-level description of the\n> API that you are proposing, and some discussion of its design and\n> tradeoffs. A big part of this description could go straight into a file\n> Documentation/technical/api-ref-transactions.txt, which will be a great\n> (and necessary) resource soon anyway.\n>\n> Michael\n>\n> [1] Though hopefully there will be future reference backends that don't\n> have to discard reflogs when a reference is deleted, so let's not bake\n> this behavior too fundamentally into the API.\n>\n> [2] ...and/or possibly one to expire reflogs for multiple references, if\n> performance would benefit significantly.\n>\n> --\n> Michael Haggerty\n> mhagger@alum.mit.edu\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"252115","messageId":"546BA21C.9030803@alum.mit.edu","threadId":"37991","inReplyTo":"CAL=YDWn1x9TMGOWrmT5KMpQ_iBR0AQ5Ej1yr1pBb4==k0-vchw@mail.gmail.com","subject":"Re: [PATCH v3 00/14] ref-transactions-reflog","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-18T19:46:36Z","receivedAt":"2014-11-18T19:46:36Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/18/2014 07:36 PM, Ronnie Sahlberg wrote:\n> On Tue, Nov 18, 2014 at 3:26 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n>> On 11/18/2014 02:35 AM, Stefan Beller wrote:\n>>> The following patch series updates the reflog handling to use transactions.\n>>> This patch series has previously been sent to the list[1].\n>>> [...]\n>>\n>> I was reviewing this patch series (I left some comments in Gerrit about\n>> the first few patches) when I realized that I'm having trouble\n>> understanding the big picture of where you want to go with this. I have\n>> the feeling that the operations that you are implementing are at too low\n>> a level of abstraction.\n>>\n>> What are the elementary write operations that are needed for a reflog?\n>> Off the top of my head,\n>>\n>> 1. Add a reflog entry when a reference is updated in a transaction.\n>> 2. Rename a reflog file when the corresponding reference is renamed.\n>> 3. Delete the reflog when the corresponding reference is deleted [1].\n>> 4. Configure a reference to be reflogged.\n>> 5. Configure a reference to not be reflogged anymore and delete any\n>>    existing reflog.\n>> 6. Selectively expire old reflog entries, e.g., based on their age.\n>>\n>> Have I forgotten any?\n>>\n>> The first three should be side-effects of the corresponding reference\n>> updates. Aside from the fact that renames are not yet done within a\n>> transaction, I think this is already the case.\n>>\n>> Number 4, I think, currently only happens in conjunction with adding a\n>> line to the reflog. So it could be implemented, say, as a\n>> FORCE_CREATE_REFLOG flag on a ref_update within a transaction.\n>>\n>> Number 5 is not very interesting, I think. For example, it could be a\n>> separate API function, disconnected from any transactions.\n>>\n>> Number 6 is more interesting, and from my quick reading, it looks like a\n>> lot of the work of this patch series is to allow number 6 to be\n>> implemented in builtin/reflog.c:expire_reflog(). But it seems to me that\n>> you are building API calls at the wrong level of abstraction. Expiring a\n>> reflog should be a single API call to the refs API, and ultimately it\n>> should be left up to the refs backend to decide how to implement it. For\n>> a filesystem-based backend, it would do what it does now. But (for\n>> example) a SQL-based backend might implement this as a single SELECT\n>> statement.\n> \n> I agree in principle. But things are more difficult since\n> expire_reflog() has very complex semantics.\n> To keep things simple for the reviews at this stage the logic is the\n> same as the original code:\n>   loop over all entries:\n>      use very complex conditionals to decide which entries to keep/remove\n>      optionally modify the sha1 values for the records we keep\n>      write records we keep back to the file one record at a time\n> \n> So that as far as possible, we keep the same rules and behavior but we\n> use a different API for the actual\n> \"write entry to new reflog\".\n> \n> \n> We could wrap this inside a new specific transaction_expire_reflog()\n> function so that other types of backends, for example an SQL backend,\n> could optimize, but I think that should be in a separate later patch\n> because expire_reflog is almost impossibly complex.\n> It will not be a simple SELECT unfortunately.\n> \n> The current expire logic is something like :\n>   1, expire all entries older than timestamp\n>   2, optionally, also expire all entries that refer to unreachable\n> objects using a different timestamp\n>       This involves actually reading the objects that the sha1 points\n> to and parsing them!\n>   3, optionally, if the sha1 objects can not be referenced, they are\n> not commit objects or if they don't exist, then expire them too.\n>       This also involves reading the objects behind the sha1.\n>   4, optionally, delete reflog entry #foo\n>   5, optionally, if any log entries were discarded due to 2,3,4 then\n> we might also re-write and modify some of the reflog entries we keep.\n> or any combination thereof\n> \n>   (6, if --dry-run is specified, just print what we would have expired)\n> \n> \n> 2 and 3 requires that we need to read the objects for the entry\n> 4 allows us to delete a specific entry\n> 5 means that even for entries we keep we will need to mutate them.\n\nThanks for the explanation. I now understand that it might be more than\na single SELECT statement.\n\nRegarding the complicated rules for expiring reflogs (1, 2, 3, 4): For\nnow I think it would be fine for the new expire_reflog() API function to\ntake a callback function as an argument.\n\nRegarding the stitching together of the survivors (5), it seems like the\nAPI function would be the right place to handle that.\n\nRegarding 6, it sounds like you could run the reflog entries through\nyour callback and report what it *would* have expired.\n\n>> I also don't have the feeling that reflog expiration has to be done\n>> within a ref_transaction. For example, is there ever a reason to combine\n>> expiration with other reference updates in a single atomic transaction?\n> \n> --updateref\n> In expire_reflog() we not only prune the reflog. When --updateref is\n> used we update the actual ref itself.\n> I think we want to have both the ref update and also the reflog update\n> both be part of a single atomic transaction.\n\nISTM that --updateref is another aspect of stitching together the\nsurviving reflog entries and could properly be done by the API\nexpire_reflog() function. Maybe the implementation would use an\n*internal* transaction. But I still don't see a need for the caller to\nbe able to combine *arbitrary* reflog changes with *arbitrary* reference\nupdates in a single transaction, and the unneeded flexibility seems to\nrequire the API to become more complicated than necessary.\n\n>> I think not.\n>>\n>> So it seems to me that it would be more practical to have a separate API\n>> function that is called to expire selected entries from a reflog [2],\n>> unconnected with any transaction.\n> \n> I think it makes the API cleaner if we have a\n> 'you can only update a ref/reflog/<other things added in the future>/\n> from within a transaction.'\n> \n> Since we need to do reflog changes within a transaction for the expire\n> reflog case as well as the rename ref case\n> I think it makes sense to enforce that reflog changes must be done\n> within a transaction to just make it consistent.\n\nI'm still not convinced. For me, \"reflog_expire()\" is an unusual outlier\noperation, much like \"git gc\" or \"git pack-refs\" or \"git fsck\". None of\nthese are part of the beautiful Git data model; they are messy\nmaintenance operations. Forcing reference transactions to be general\nenough to allow reflog expiration to be implemented *outside* the refs\nAPI sacrificies their simplicity for lots of infrastructure that will\nprobably only be used to implement this single operation. Better to\nimplement reflog expiration *inside* the refs API.\n\nThat's my take on it, anyway.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"252123","messageId":"xmqqr3x0uu81.fsf@gitster.dls.corp.google.com","threadId":"37991","inReplyTo":"546BA21C.9030803@alum.mit.edu","subject":"Re: [PATCH v3 00/14] ref-transactions-reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-18T20:30:54Z","receivedAt":"2014-11-18T20:30:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> I'm still not convinced. For me, \"reflog_expire()\" is an unusual outlier\n> operation, much like \"git gc\" or \"git pack-refs\" or \"git fsck\". None of\n> these are part of the beautiful Git data model; they are messy\n> maintenance operations. Forcing reference transactions to be general\n> enough to allow reflog expiration to be implemented *outside* the refs\n> API sacrificies their simplicity for lots of infrastructure that will\n> probably only be used to implement this single operation. Better to\n> implement reflog expiration *inside* the refs API.\n\nSorry, but I lost track---which one is inside and which one is\noutside?\n"},{"id":"252133","messageId":"546BB722.5020901@alum.mit.edu","threadId":"37991","inReplyTo":"xmqqr3x0uu81.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 00/14] ref-transactions-reflog","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-18T21:16:18Z","receivedAt":"2014-11-18T21:16:18Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/18/2014 09:30 PM, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n>> I'm still not convinced. For me, \"reflog_expire()\" is an unusual outlier\n>> operation, much like \"git gc\" or \"git pack-refs\" or \"git fsck\". None of\n>> these are part of the beautiful Git data model; they are messy\n>> maintenance operations. Forcing reference transactions to be general\n>> enough to allow reflog expiration to be implemented *outside* the refs\n>> API sacrificies their simplicity for lots of infrastructure that will\n>> probably only be used to implement this single operation. Better to\n>> implement reflog expiration *inside* the refs API.\n> \n> Sorry, but I lost track---which one is inside and which one is\n> outside?\n\nBy \"inside\" I mean the code that would be within the reference-handling\nlibrary if we had such a thing; i.e., implemented in refs.c. By\n\"outside\" I mean in the code that calls the library; in this case the\n\"outside\" code would live in builtin/reflog.c.\n\nIn other words, I'd prefer the \"outside\" code in builtin/reflog.c to\nlook vaguely like\n\n    expire_reflogs_for_me_please(refname,\n                                 should_expire_cb, cbdata, flags)\n\nrather than\n\n    transaction = ...\n    for_each_reflog_entry {\n        if should_expire()\n            adjust neighbor reflog entries if necessary (actually,\n                   they're transaction entries so we would have to\n                   preprocess them before putting them in the\n                   transaction)\n        else\n            add reflog entry to transaction\n    }\n    ref_transaction_commit()\n\nand instead handle as much of the iteration, bookkeeping, and rewriting\nas possible inside expire_reflogs_for_me_please().\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"252137","messageId":"xmqqsihgtcyx.fsf@gitster.dls.corp.google.com","threadId":"37991","inReplyTo":"546BB722.5020901@alum.mit.edu","subject":"Re: [PATCH v3 00/14] ref-transactions-reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-18T21:28:54Z","receivedAt":"2014-11-18T21:28:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n>> Sorry, but I lost track---which one is inside and which one is\n>> outside?\n>\n> By \"inside\" I mean the code that would be within the reference-handling\n> library if we had such a thing; i.e., implemented in refs.c. By\n> \"outside\" I mean in the code that calls the library; in this case the\n> \"outside\" code would live in builtin/reflog.c.\n>\n> In other words, I'd prefer the \"outside\" code in builtin/reflog.c to\n> look vaguely like\n>\n>     expire_reflogs_for_me_please(refname,\n>                                  should_expire_cb, cbdata, flags)\n>\n> rather than\n> ... (written as a client of the ref API) ...\n\nOK, I very much agree with that.\n"},{"id":"252224","messageId":"CAGZ79kb3DOrL_txW-qxzd0=4sKrOiPTdSg-17_0+__wuj0TBaQ@mail.gmail.com","threadId":"37991","inReplyTo":"xmqqsihgtcyx.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 00/14] ref-transactions-reflog","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-19T23:22:01Z","receivedAt":"2014-11-19T23:22:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Sorry for the long delay.\nThanks for the explanation and discussion.\n\nSo do I understand it right, that you are not opposing\nthe introduction of \"everything should go through transactions\"\nbut rather the detail and abstraction level of the API?\n\nSo starting from Michaels proposal in the first response:\n\n1. Add a reflog entry when a reference is updated in a transaction.\n\nok\n\n2. Rename a reflog file when the corresponding reference is renamed.\n\nThis should happen within the same transaction as the reference is\nrenamed, right?\nSo we don't have a multistep process here, which may abort in between having the\nreference updated and a broken reflog or vice versa. We want to either\nhave both\nthe ref and the reflog updated or neither.\n\n3. Delete the reflog when the corresponding reference is deleted [1].\n\nalso as one transaction?\n\n4. Configure a reference to be reflogged.\n5. Configure a reference to not be reflogged anymore and delete any\n   existing reflog.\n\nWhy do we need 4 and 5 here? Wouldn't all refs be reflog by default and\nwhy do I want to exclude some?\n\n6. Selectively expire old reflog entries, e.g., based on their age.\n\nThis is the maintenance operation, which you were talking about.\nIn my vision, this also should go into one transaction. So you have the\nbusiness logic figuring out all the changes (\"drop reflog entry a b and d\")\nand within one transaction we can perform all of the changes.\n\nThanks,\nStefan\n\n\nOn Tue, Nov 18, 2014 at 1:28 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n>\n>>> Sorry, but I lost track---which one is inside and which one is\n>>> outside?\n>>\n>> By \"inside\" I mean the code that would be within the reference-handling\n>> library if we had such a thing; i.e., implemented in refs.c. By\n>> \"outside\" I mean in the code that calls the library; in this case the\n>> \"outside\" code would live in builtin/reflog.c.\n>>\n>> In other words, I'd prefer the \"outside\" code in builtin/reflog.c to\n>> look vaguely like\n>>\n>>     expire_reflogs_for_me_please(refname,\n>>                                  should_expire_cb, cbdata, flags)\n>>\n>> rather than\n>> ... (written as a client of the ref API) ...\n>\n> OK, I very much agree with that.\n"},{"id":"252240","messageId":"20141120032453.GH6527@google.com","threadId":"37991","inReplyTo":"CAGZ79kb3DOrL_txW-qxzd0=4sKrOiPTdSg-17_0+__wuj0TBaQ@mail.gmail.com","subject":"Re: [PATCH v3 00/14] ref-transactions-reflog","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-11-20T03:24:53Z","receivedAt":"2014-11-20T03:24:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nStefan Beller wrote:\n\n> Sorry for the long delay.\n> Thanks for the explanation and discussion.\n>\n> So do I understand it right, that you are not opposing\n> the introduction of \"everything should go through transactions\"\n> but rather the detail and abstraction level of the API?\n\nFor what it's worth, I don't personally think it makes sense to put\nthe options supported by 'git reflog expire' into the transaction API\nas top-level functions.\n\nInstead, I think it makes sense, to start off, to support the same\nbuilding block operations that are used in the current file-based\ncode.  That may mean having an API that can't express tricks that e.g.\nan SQL-based backend would be able to optimize (removing some items\nfrom a reflog without copying the rest, filtering based on conditions\nthat can be expressed in SQL such as date, etc) but I think it's fine\nas a starting point.  Later we can add new operations, change existing\nones, and so on, based on experience with real backends.\n\nThe write operations for file-based reflog handling are simple:\n\n\t- create a new reflog with a single reflog entry\n\n\t- add an entry to an existing reflog\n\n\t- (optional) copy a reflog wholesale --- this can be\n\t  implemented in terms of \"add an entry\", but copying in\n\t  blocks (or making a reflink, on filesystems that support\n\t  that) can make this faster\n\n\t- remove a reflog\n\nThe reflog bookkeeping involved in renaming a ref can be implemented\nas copy + delete.\n\nI also have some thoughts about how those operations can be\nimplemented without such a performance hit (reading the whole reflog\ninto memory as part of the transaction seems problematic to me), but\nthat should probably wait for a separate message (and I've talked\nabout it a bit in person).\n\n[...]\n> 4. Configure a reference to be reflogged.\n> 5. Configure a reference to not be reflogged anymore and delete any\n>    existing reflog.\n>\n> Why do we need 4 and 5 here? Wouldn't all refs be reflog by default and\n> why do I want to exclude some?\n\nSee --create-reflog in git-branch(1) and core.logallrefupdates in\ngit-config(1).\n\nReflogs are disabled by default in bare repositories, which makes it\neasier for unnecessary objects on a server to be more promptly removed\nby gcs after a non-fast-forward push.  I prefer to turn on reflogs\nwhen setting up a git server for my personal use.  It might be worth\nflipping that default (as an orthogonal change).\n\n> 6. Selectively expire old reflog entries, e.g., based on their age.\n>\n> This is the maintenance operation, which you were talking about.\n> In my vision, this also should go into one transaction. So you have the\n> business logic figuring out all the changes (\"drop reflog entry a b and d\")\n> and within one transaction we can perform all of the changes.\n\nMakes sense.\n\nThanks,\nJonathan\n"},{"id":"252248","messageId":"546DC8E9.6090504@alum.mit.edu","threadId":"37991","inReplyTo":"CAGZ79kb3DOrL_txW-qxzd0=4sKrOiPTdSg-17_0+__wuj0TBaQ@mail.gmail.com","subject":"Re: [PATCH v3 00/14] ref-transactions-reflog","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-20T10:56:41Z","receivedAt":"2014-11-20T10:56:41Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/20/2014 12:22 AM, Stefan Beller wrote:\n> Sorry for the long delay.\n> Thanks for the explanation and discussion.\n> \n> So do I understand it right, that you are not opposing\n> the introduction of \"everything should go through transactions\"\n> but rather the detail and abstraction level of the API?\n\nCorrect.\n\n> So starting from Michaels proposal in the first response:\n> \n> 1. Add a reflog entry when a reference is updated in a transaction.\n> \n> ok\n> \n> 2. Rename a reflog file when the corresponding reference is renamed.\n> \n> This should happen within the same transaction as the reference is\n> renamed, right?\n\nYes. Maybe there should be a \"rename reference\" operation that can be\nadded to a transaction, and it simply knows to rename any associated\nreflogs. Then the calling code wouldn't have to worry about reflogs\nexplicitly in this case at all.\n\n> So we don't have a multistep process here, which may abort in between having the\n> reference updated and a broken reflog or vice versa. We want to either\n> have both\n> the ref and the reflog updated or neither.\n\nYes.\n\n> 3. Delete the reflog when the corresponding reference is deleted [1].\n> \n> also as one transaction?\n\nIt would be a side-effect of committing a transaction that contains a\nreference deletion. The deletion of the reflog would be done at the same\ntime that the rest of the transaction is committed, and again the\ncalling code wouldn't have to explicitly worry about the reflogs.\n\n> 4. Configure a reference to be reflogged.\n> 5. Configure a reference to not be reflogged anymore and delete any\n>    existing reflog.\n> \n> Why do we need 4 and 5 here? Wouldn't all refs be reflog by default and\n> why do I want to exclude some?\n> \n> 6. Selectively expire old reflog entries, e.g., based on their age.\n> \n> This is the maintenance operation, which you were talking about.\n> In my vision, this also should go into one transaction. So you have the\n> business logic figuring out all the changes (\"drop reflog entry a b and d\")\n> and within one transaction we can perform all of the changes.\n\nBut if we take the approach described above, AFAICT this operation is\nthe only one that would require the caller to manipulate reflog entries\nexplicitly. And it has to iterate through the old reflog entries, decide\nwhich ones to keep, possibly change its neighbors to eliminate gaps in\nthe chain, then stuff each of the reflog entries into a transaction one\nby one. To allow this to be implemented on the caller side, the\ntransaction API has to be complicated in the following ways:\n\n* Add a transaction_update_type (UPDATE_SHA1 vs. UPDATE_LOG).\n* Add reflog_fd, reflog_lock, and committer members to struct ref_update.\n* New function transaction_update_reflog().\n* A new flag REFLOG_TRUNCATE that allows the reflog file to be truncated\nbefore writing.\n* Machinery that recognizes that a transaction contains multiple reflog\nupdates for the same reference and processes them specially to avoid\nlocking and rewriting the reflog file multiple times.\n\nSo this design has the caller serializing all reflog entries into\nseparate ref_update structs (which implies that they are held in RAM!)\nonly for ref_transaction_commit() to scan through all ref_updates\nlooking for reflog updates that go together so that they can be\nprocessed as a whole. In other words, the caller picks the reflog apart\nand then ref_transaction_commit() glues it back together. It's all very\ncontrived.\n\nI suggest that the caller only be responsible for deciding which reflog\nentries to keep (by supplying a callback function), and a new\nexpire_reflogs_for_me_please() API function be responsible for taking\nout a lock, feeding the old reflog entries to the callback, expiring the\nunwanted entries, optionally eliminating gaps in the chain (for the use\nof \"reflog [expire|delete] --rewrite\"), writing the new reflog entries,\nand optionally updating the reference itself (for the use of \"reflog\n[expire|delete] --updateref\").\n\nThe benefit will be simpler code, a better separation of\nresponsibilities, and a simpler VTABLE that future reference backends\nhave to implement.\n\nI would love to work on this but unfortunately have way too much on my\nplate right now.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"252267","messageId":"xmqqa93l4vzq.fsf@gitster.dls.corp.google.com","threadId":"37991","inReplyTo":"20141120032453.GH6527@google.com","subject":"Re: [PATCH v3 00/14] ref-transactions-reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-20T17:34:01Z","receivedAt":"2014-11-20T17:34:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> I also have some thoughts about how those operations can be\n> implemented without such a performance hit (reading the whole\n> reflog into memory as part of the transaction seems problematic to\n> me), but that should probably wait for a separate message (and\n> I've talked about it a bit in person).\n\nPerhaps iterator interface such as for_each_reflog_ent() would help?\n"},{"id":"252276","messageId":"20141120181701.GB15945@google.com","threadId":"37991","inReplyTo":"546DC8E9.6090504@alum.mit.edu","subject":"Re: [PATCH v3 00/14] ref-transactions-reflog","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-11-20T18:17:01Z","receivedAt":"2014-11-20T18:17:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Michael Haggerty wrote:\n> On 11/20/2014 12:22 AM, Stefan Beller wrote:\n\n>> 3. Delete the reflog when the corresponding reference is deleted [1].\n>>\n>> also as one transaction?\n>\n> It would be a side-effect of committing a transaction that contains a\n> reference deletion. The deletion of the reflog would be done at the same\n> time that the rest of the transaction is committed, and again the\n> calling code wouldn't have to explicitly worry about the reflogs.\n\nThere is \"git reflog delete <ref>\", for when you have logallrefupdates\ndisabled and had explicitly enabled reflogs for a particular ref and\nnow want to turn it off.\n\n[...]\n> So this design has the caller serializing all reflog entries into\n> separate ref_update structs (which implies that they are held in RAM!)\n> only for ref_transaction_commit() to scan through all ref_updates\n> looking for reflog updates that go together so that they can be\n> processed as a whole. In other words, the caller picks the reflog apart\n> and then ref_transaction_commit() glues it back together. It's all very\n> contrived.\n\nI think there is a simpler and more efficient way to implement this.\n\ntransaction_update_reflog() can append to a .lock file.\ntransaction_commit() then would rename it into place.\n\nThere is some fuss about naming the .lock file to avoid D/F conflicts,\nwhich is a topic for a separate message.\n\n> I suggest that the caller only be responsible for deciding which reflog\n> entries to keep (by supplying a callback function),\n\nThat could be handy.  The basic operations described before would still\nbe needed, though:\n\n\tcreate a new reflog with one entry, for new refs\n\n\tappend an entry to a reflog, for ref updates (and the associated\n\t\tsymref reflog update)\n\n\tcopy (or rename --- that's a more minor detail) a reflog, for\n\t\trenaming refs\n\n\tdelete a reflog, for \"git reflog delete\"\n\nAnd the \"filter reflog\" operation you are describing is implementable\nusing those four operations, with no performance hit when dealing with\nreflogs stored in the files backend.\n\nProviding these operations doesn't prevent adding \"filter reflog using\ncallback\" later if it turns out to be the right operation for other\nbackends.  It could turn out that some other primitive operations that\nare easy as an SQL operation is more useful, like \"delete reflog\nentry\" (without iterating over the others) or \"expire entries older\nthan <date>\".  The nice thing is that adding those wouldn't break any\ncode using the initial four operations described above.  So this seems\nlike a good starting point.\n\n[...]\n> I would love to work on this but unfortunately have way too much on my\n> plate right now.\n\nOf course code is always an easy way to change my mind, when the time\ncomes. ;-)\n\nThanks,\nJonathan\n"},{"id":"252631","messageId":"1417066485-24921-1-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"20141120181701.GB15945@google.com","subject":"[PATCH 0/4] Using transactions for the reflog","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-27T05:34:41Z","receivedAt":"2014-11-27T05:34:41Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This is the core part of the refs-transactions-reflog series[1],\nwhich was in discussion for a bit already.\n\nThe idea is to have the reflog being part of the transactions, which\nthe refs are already using, so the we're moving towards a database\nlike API in the long run. This makes git easier to maintain as well \nas opening the possibility to replace the backend with a real database.\n\nThe first patch is essentially just some sed magic with reformatting\nthe code, so the naming convention fits better, because the transactions \nwill handle both the refs as well as the reflog after this series. \n\nThe second patch introduces a new enum field to indicate, if we deal with\na ref or with a reflog entry in the transaction. \n\nThe meat and most of the lines of code are found in the 3rd patch.\nWe introduce a rather lengthy function transaction_update_reflog,\nwhich prepares all the reflog related changes.\nThe transaction_commit function will then also put the reflog changes\nin place in a \"best effort\" atomic way.\nUnlike in previous versions, we don't keep all the reflog in memory,\nbut use a temporary file in $GIT_DIR instead and the update can be done\nusing an atomic rename(...).\n\nOne of my todos is to make the error handling in the transaction_update_reflog\nfunction a bit less repetitive either during the discussion of this series\nor as a follow up.\n\nThe last patch in this series makes use of the transaction system in the \nuser facing code, when running \"git reflog expire\" for example. \n\nI'd appreciate any comments. \n\nApart from sending feedback on the list, you can find this series \nat github[2] embedded into the longer version of the series.\nIn that series at github there are a few more patches[3], which are already\nreviewed and residing in Junios repository or considered trivial cleanups.\n\nThanks,\nStefan\n\n[1] http://comments.gmane.org/gmane.comp.version-control.git/259712\n[2] https://github.com/stefanbeller/git/commits/todo_sb13_ref-transactions-reflog-as-file\n[3] The first 2 commits on top of Git 2.2-rc3 are origin/sb/ref-transaction-unify-to-update, \n    the third is in origin/sb/log-ref-write-fd, then comes this series in 4 patches. \n    The remaining latest 4 patches are clean up patches, mainly removing parts from the refs API\n    which are no longer in use. I do not include these in this patch series, as I don't want to\n    scare people away with a huge bulk of messages.\n\nRonnie Sahlberg (4):\n  refs.c: rename the transaction functions\n  refs.c: add a new update_type field to ref_update\n  refs.c: add a transaction function to append a reflog entry\n  reflog.c: use a reflog transaction when writing during expire\n\n branch.c               |  13 +--\n builtin/commit.c       |  10 +-\n builtin/fetch.c        |  12 +--\n builtin/receive-pack.c |  13 ++-\n builtin/reflog.c       |  85 ++++++++---------\n builtin/replace.c      |  10 +-\n builtin/tag.c          |  10 +-\n builtin/update-ref.c   |  26 ++---\n fast-import.c          |  22 ++---\n refs.c                 | 251 ++++++++++++++++++++++++++++++++++++++++---------\n refs.h                 |  57 +++++++----\n sequencer.c            |  12 +--\n walker.c               |  10 +-\n 13 files changed, 352 insertions(+), 179 deletions(-)\n\n-- \n2.2.0.rc3\n"},{"id":"252634","messageId":"1417066485-24921-2-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1417066485-24921-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/4] refs.c: rename the transaction functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-27T05:34:42Z","receivedAt":"2014-11-27T05:34:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nRename the transaction functions. Remove the leading ref_ from the\nnames and append _ref to the names for functions that create/delete/\nupdate sha1 refs.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\nThis commit can be reproduced via\nfind . -name \"*.[ch]\" -print | xargs sed -i ' \\\n\ts/REF_TRANSACTION_OPEN/TRANSACTION_OPEN/; \\\n\ts/REF_TRANSACTION_CLOSED/TRANSACTION_CLOSED/; \\\n\ts/ref_transaction_begin/transaction_begin/; \\\n\ts/ref_transaction_commit/transaction_commit/; \\\n\ts/ref_transaction_create/transaction_create_ref/; \\\n\ts/ref_transaction_delete/transaction_delete_ref/; \\\n\ts/ref_transaction_free/transaction_free/; \\\n\ts/ref_transaction_update/transaction_update_ref/; \\\n\ts/ref_transaction/transaction/'\nmodulo white space changes for alignment.\n\n---\n branch.c               | 13 +++++----\n builtin/commit.c       | 10 +++----\n builtin/fetch.c        | 12 ++++----\n builtin/receive-pack.c | 13 ++++-----\n builtin/replace.c      | 10 +++----\n builtin/tag.c          | 10 +++----\n builtin/update-ref.c   | 26 ++++++++---------\n fast-import.c          | 22 +++++++-------\n refs.c                 | 78 +++++++++++++++++++++++++-------------------------\n refs.h                 | 36 +++++++++++------------\n sequencer.c            | 12 ++++----\n walker.c               | 10 +++----\n 12 files changed, 126 insertions(+), 126 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 4bab55a..c8462de 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -279,16 +279,17 @@ void create_branch(const char *head,\n \t\tlog_all_ref_updates = 1;\n \n \tif (!dont_change_ref) {\n-\t\tstruct ref_transaction *transaction;\n+\t\tstruct 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_ref(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\tstrbuf_release(&err);\n \t}\n \ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e108c53..f50b7df 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1673,7 +1673,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstruct stat statbuf;\n \tstruct commit *current_head = NULL;\n \tstruct commit_extra_header *extra = NULL;\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n@@ -1804,17 +1804,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    transaction_update_ref(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_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 7b84d35..0be0b09 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -404,7 +404,7 @@ static int s_update_ref(const char *action,\n {\n \tchar msg[1024];\n \tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \tint ret, df_conflict = 0;\n \n@@ -414,23 +414,23 @@ 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    transaction_update_ref(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) {\n \t\tdf_conflict = (ret == TRANSACTION_NAME_CONFLICT);\n \t\tgoto fail;\n \t}\n \n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \tstrbuf_release(&err);\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 32fc540..397abc9 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -838,26 +838,25 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t}\n \telse {\n \t\tstruct strbuf err = STRBUF_INIT;\n-\t\tstruct ref_transaction *transaction;\n+\t\tstruct transaction *transaction;\n \n \t\tif (shallow_update && si->shallow_ref[cmd->index] &&\n \t\t    update_shallow_ref(cmd, si))\n \t\t\treturn \"shallow error\";\n \n-\t\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    transaction_update_ref(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\tref_transaction_free(transaction);\n-\n+\t\t    transaction_commit(transaction, &err)) {\n+\t\t\ttransaction_free(transaction);\n \t\t\trp_error(\"%s\", err.buf);\n \t\t\tstrbuf_release(&err);\n \t\t\treturn \"failed to update ref\";\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 85d39b5..5a7ab1f 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -155,7 +155,7 @@ static int replace_object_sha1(const char *object_ref,\n \tunsigned char prev[20];\n \tenum object_type obj_type, repl_type;\n \tchar ref[PATH_MAX];\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n \tobj_type = sha1_object_info(object, NULL);\n@@ -169,14 +169,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    transaction_update_ref(transaction, ref, repl, prev,\n \t\t\t\t   0, 1, NULL, &err) ||\n-\t    ref_transaction_commit(transaction, &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 e633f4e..5f3554b 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -583,7 +583,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tconst char *msgfile = NULL, *keyid = NULL;\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n \tstruct commit_list *with_commit = NULL;\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n@@ -730,13 +730,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    transaction_update_ref(transaction, ref.buf, object, prev,\n \t\t\t\t   0, 1, NULL, &err) ||\n-\t    ref_transaction_commit(transaction, &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 6c9be05..af08dd9 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -175,7 +175,7 @@ static int parse_next_sha1(struct strbuf *input, const char **next,\n  * depending on how line_termination is set.\n  */\n \n-static const char *parse_cmd_update(struct ref_transaction *transaction,\n+static const char *parse_cmd_update(struct transaction *transaction,\n \t\t\t\t    struct strbuf *input, const char *next)\n {\n \tstruct strbuf err = STRBUF_INIT;\n@@ -198,7 +198,7 @@ static const char *parse_cmd_update(struct ref_transaction *transaction,\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+\tif (transaction_update_ref(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@@ -209,7 +209,7 @@ static const char *parse_cmd_update(struct ref_transaction *transaction,\n \treturn next;\n }\n \n-static const char *parse_cmd_create(struct ref_transaction *transaction,\n+static const char *parse_cmd_create(struct transaction *transaction,\n \t\t\t\t    struct strbuf *input, const char *next)\n {\n \tstruct strbuf err = STRBUF_INIT;\n@@ -229,7 +229,7 @@ static const char *parse_cmd_create(struct ref_transaction *transaction,\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+\tif (transaction_create_ref(transaction, refname, new_sha1,\n \t\t\t\t   update_flags, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n@@ -240,7 +240,7 @@ static const char *parse_cmd_create(struct ref_transaction *transaction,\n \treturn next;\n }\n \n-static const char *parse_cmd_delete(struct ref_transaction *transaction,\n+static const char *parse_cmd_delete(struct transaction *transaction,\n \t\t\t\t    struct strbuf *input, const char *next)\n {\n \tstruct strbuf err = STRBUF_INIT;\n@@ -264,7 +264,7 @@ static const char *parse_cmd_delete(struct ref_transaction *transaction,\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+\tif (transaction_delete_ref(transaction, refname, old_sha1,\n \t\t\t\t   update_flags, have_old, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n@@ -275,7 +275,7 @@ static const char *parse_cmd_delete(struct ref_transaction *transaction,\n \treturn next;\n }\n \n-static const char *parse_cmd_verify(struct ref_transaction *transaction,\n+static const char *parse_cmd_verify(struct transaction *transaction,\n \t\t\t\t    struct strbuf *input, const char *next)\n {\n \tstruct strbuf err = STRBUF_INIT;\n@@ -300,7 +300,7 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction,\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+\tif (transaction_update_ref(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@@ -320,7 +320,7 @@ static const char *parse_cmd_option(struct strbuf *input, const char *next)\n \treturn next + 8;\n }\n \n-static void update_refs_stdin(struct ref_transaction *transaction)\n+static void update_refs_stdin(struct transaction *transaction)\n {\n \tstruct strbuf input = STRBUF_INIT;\n \tconst char *next;\n@@ -376,9 +376,9 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \n \tif (read_stdin) {\n \t\tstruct strbuf err = STRBUF_INIT;\n-\t\tstruct ref_transaction *transaction;\n+\t\tstruct transaction *transaction;\n \n-\t\ttransaction = ref_transaction_begin(&err);\n+\t\ttransaction = transaction_begin(&err);\n \t\tif (!transaction)\n \t\t\tdie(\"%s\", err.buf);\n \t\tif (delete || no_deref || argc > 0)\n@@ -386,9 +386,9 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin(transaction);\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\tstrbuf_release(&err);\n \t\treturn 0;\n \t}\ndiff --git a/fast-import.c b/fast-import.c\nindex d0bd285..152d944 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1687,7 +1687,7 @@ found_entry:\n static int update_branch(struct branch *b)\n {\n \tstatic const char *msg = \"fast-import\";\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tunsigned char old_sha1[20];\n \tstruct strbuf err = STRBUF_INIT;\n \n@@ -1713,17 +1713,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    transaction_update_ref(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_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 \tstrbuf_release(&err);\n \treturn 0;\n }\n@@ -1745,9 +1745,9 @@ static void dump_tags(void)\n \tstruct tag *t;\n \tstruct strbuf ref_name = STRBUF_INIT;\n \tstruct strbuf err = STRBUF_INIT;\n-\tstruct ref_transaction *transaction;\n+\tstruct 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@@ -1756,17 +1756,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\tif (transaction_update_ref(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 150c980..f0f0d23 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -26,7 +26,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_ref when a loose ref is being\n  * pruned.\n  */\n #define REF_ISPRUNING\t0x0100\n@@ -2533,23 +2533,23 @@ static void try_remove_empty_parents(char *name)\n /* make sure nobody touched the ref, and unlink */\n static void prune_ref(struct ref_to_prune *r)\n {\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n \tif (check_refname_format(r->name, 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    transaction_delete_ref(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_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 \tstrbuf_release(&err);\n \ttry_remove_empty_parents(r->name);\n }\n@@ -2711,20 +2711,20 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\n \n int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n {\n-\tstruct ref_transaction *transaction;\n+\tstruct 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    transaction_delete_ref(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_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 \tstrbuf_release(&err);\n \treturn 0;\n }\n@@ -3543,9 +3543,9 @@ struct ref_update {\n  *         an active transaction or if there is a failure while building\n  *         the transaction thus rendering it failed/inactive.\n  */\n-enum ref_transaction_state {\n-\tREF_TRANSACTION_OPEN   = 0,\n-\tREF_TRANSACTION_CLOSED = 1\n+enum transaction_state {\n+\tTRANSACTION_OPEN   = 0,\n+\tTRANSACTION_CLOSED = 1\n };\n \n /*\n@@ -3553,21 +3553,21 @@ enum ref_transaction_state {\n  * consist of checks and updates to multiple references, carried out\n  * as atomically as possible.  This structure is opaque to callers.\n  */\n-struct ref_transaction {\n+struct transaction {\n \tstruct ref_update **updates;\n \tsize_t alloc;\n \tsize_t nr;\n-\tenum ref_transaction_state state;\n+\tenum transaction_state state;\n };\n \n-struct ref_transaction *ref_transaction_begin(struct strbuf *err)\n+struct transaction *transaction_begin(struct strbuf *err)\n {\n \tassert(err);\n \n-\treturn xcalloc(1, sizeof(struct ref_transaction));\n+\treturn xcalloc(1, sizeof(struct transaction));\n }\n \n-void ref_transaction_free(struct ref_transaction *transaction)\n+void transaction_free(struct transaction *transaction)\n {\n \tint i;\n \n@@ -3582,7 +3582,7 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \tfree(transaction);\n }\n \n-static struct ref_update *add_update(struct ref_transaction *transaction,\n+static struct ref_update *add_update(struct transaction *transaction,\n \t\t\t\t     const char *refname)\n {\n \tsize_t len = strlen(refname);\n@@ -3594,7 +3594,7 @@ 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_ref(struct 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@@ -3605,7 +3605,7 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \n \tassert(err);\n \n-\tif (transaction->state != REF_TRANSACTION_OPEN)\n+\tif (transaction->state != TRANSACTION_OPEN)\n \t\tdie(\"BUG: update called for transaction that is not open\");\n \n \tif (have_old && !old_sha1)\n@@ -3629,23 +3629,23 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \treturn 0;\n }\n \n-int ref_transaction_create(struct ref_transaction *transaction,\n+int transaction_create_ref(struct 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-\treturn ref_transaction_update(transaction, refname, new_sha1,\n+\treturn transaction_update_ref(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+int transaction_delete_ref(struct 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-\treturn ref_transaction_update(transaction, refname, null_sha1,\n+\treturn transaction_update_ref(transaction, refname, null_sha1,\n \t\t\t\t      old_sha1, flags, have_old, msg, err);\n }\n \n@@ -3653,17 +3653,17 @@ int update_ref(const char *action, const char *refname,\n \t       const unsigned char *sha1, const unsigned char *oldval,\n \t       int flags, enum action_on_err onerr)\n {\n-\tstruct ref_transaction *t;\n+\tstruct 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    transaction_update_ref(t, refname, sha1, oldval, flags,\n \t\t\t\t   !!oldval, action, &err) ||\n-\t    ref_transaction_commit(t, &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);\n@@ -3678,7 +3678,7 @@ int update_ref(const char *action, const char *refname,\n \t\treturn 1;\n \t}\n \tstrbuf_release(&err);\n-\tref_transaction_free(t);\n+\ttransaction_free(t);\n \treturn 0;\n }\n \n@@ -3706,8 +3706,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 transaction *transaction,\n+\t\t       struct strbuf *err)\n {\n \tint ret = 0, delnum = 0, i;\n \tconst char **delnames;\n@@ -3716,11 +3716,11 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \n \tassert(err);\n \n-\tif (transaction->state != REF_TRANSACTION_OPEN)\n+\tif (transaction->state != TRANSACTION_OPEN)\n \t\tdie(\"BUG: commit called for transaction that is not open\");\n \n \tif (!n) {\n-\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\t\ttransaction->state = TRANSACTION_CLOSED;\n \t\treturn 0;\n \t}\n \n@@ -3799,7 +3799,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \tclear_loose_ref_cache(&ref_cache);\n \n cleanup:\n-\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\ttransaction->state = TRANSACTION_CLOSED;\n \n \tfor (i = 0; i < n; i++)\n \t\tif (updates[i]->lock)\ndiff --git a/refs.h b/refs.h\nindex 7d675b7..556adfd 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -11,22 +11,22 @@ struct ref_lock {\n };\n \n /*\n- * A ref_transaction represents a collection of ref updates\n+ * A transaction represents a collection of ref updates\n  * that should succeed or fail together.\n  *\n  * Calling sequence\n  * ----------------\n- * - Allocate and initialize a `struct ref_transaction` by calling\n- *   `ref_transaction_begin()`.\n+ * - Allocate and initialize a `struct transaction` by calling\n+ *   `transaction_begin()`.\n  *\n  * - List intended ref updates by calling functions like\n- *   `ref_transaction_update()` and `ref_transaction_create()`.\n+ *   `transaction_update_ref()` and `transaction_create_ref()`.\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@@ -42,7 +42,7 @@ struct ref_lock {\n  * The message is appended to err without first clearing err.\n  * err will not be '\\n' terminated.\n  */\n-struct ref_transaction;\n+struct transaction;\n \n /*\n  * Bit values set in the flags argument passed to each_ref_fn():\n@@ -181,8 +181,8 @@ extern int is_branch(const char *refname);\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_ref(),\n+ * transaction_create_ref(), etc.\n  * REF_NODEREF: act on the ref directly, instead of dereferencing\n  *              symbolic references.\n  * REF_DELETING: tolerate broken refs\n@@ -269,13 +269,13 @@ 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 transaction *transaction_begin(struct strbuf *err);\n \n /*\n  * The following functions add a reference check or update to a\n- * ref_transaction.  In all of them, refname is the name of the\n+ * transaction.  In all of them, refname is the name of the\n  * reference to be affected.  The functions make internal copies of\n  * refname and msg, so the caller retains ownership of these parameters.\n  * flags can be REF_NODEREF; it is passed to update_ref_lock().\n@@ -291,7 +291,7 @@ 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+int transaction_update_ref(struct 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@@ -307,7 +307,7 @@ 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+int transaction_create_ref(struct 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@@ -321,7 +321,7 @@ 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+int transaction_delete_ref(struct 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@@ -337,13 +337,13 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n #define TRANSACTION_NAME_CONFLICT -1\n /* All other errors. */\n #define TRANSACTION_GENERIC_ERROR -2\n-int ref_transaction_commit(struct ref_transaction *transaction,\n-\t\t\t   struct strbuf *err);\n+int transaction_commit(struct 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 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 a03d4fa..f888005 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -238,7 +238,7 @@ static int error_dirty_index(struct replay_opts *opts)\n static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \t\t\tint unborn, struct replay_opts *opts)\n {\n-\tstruct ref_transaction *transaction;\n+\tstruct transaction *transaction;\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf err = STRBUF_INIT;\n \n@@ -248,13 +248,13 @@ 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\",\n+\t    transaction_update_ref(transaction, \"HEAD\",\n \t\t\t\t   to, unborn ? null_sha1 : from,\n \t\t\t\t   0, 1, sb.buf, &err) ||\n-\t    ref_transaction_commit(transaction, &err)) {\n-\t\tref_transaction_free(transaction);\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@@ -263,7 +263,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \n \tstrbuf_release(&sb);\n \tstrbuf_release(&err);\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \treturn 0;\n }\n \ndiff --git a/walker.c b/walker.c\nindex f149371..f1d5e9b 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -253,7 +253,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n {\n \tstruct strbuf refname = STRBUF_INIT;\n \tstruct strbuf err = STRBUF_INIT;\n-\tstruct ref_transaction *transaction = NULL;\n+\tstruct transaction *transaction = NULL;\n \tunsigned char *sha1 = xmalloc(targets * 20);\n \tchar *msg = NULL;\n \tint i, ret = -1;\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 done;\n@@ -298,7 +298,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \t\t\tcontinue;\n \t\tstrbuf_reset(&refname);\n \t\tstrbuf_addf(&refname, \"refs/%s\", write_ref[i]);\n-\t\tif (ref_transaction_update(transaction, refname.buf,\n+\t\tif (transaction_update_ref(transaction, refname.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@@ -306,7 +306,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \t\t\tgoto done;\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 \t\tgoto done;\n \t}\n@@ -314,7 +314,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \tret = 0;\n \n done:\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \tfree(msg);\n \tfree(sha1);\n \tstrbuf_release(&err);\n-- \n2.2.0.rc3\n"},{"id":"252632","messageId":"1417066485-24921-3-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1417066485-24921-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/4] refs.c: add a new update_type field to ref_update","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-27T05:34:43Z","receivedAt":"2014-11-27T05:34:43Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nAdd 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 <sahlberg@google.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n refs.c | 27 +++++++++++++++++++++++----\n 1 file changed, 23 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex f0f0d23..84e086f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3516,6 +3516,10 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n \treturn retval;\n }\n \n+enum transaction_update_type {\n+\tUPDATE_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@@ -3523,6 +3527,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@@ -3583,12 +3588,14 @@ void transaction_free(struct transaction *transaction)\n }\n \n static struct ref_update *add_update(struct 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@@ -3618,7 +3625,7 @@ int transaction_update_ref(struct transaction *transaction,\n \t\treturn -1;\n \t}\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@@ -3696,13 +3703,17 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \n \tassert(err);\n \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\tstrbuf_addf(err,\n \t\t\t\t    \"Multiple updates for ref '%s' not allowed.\",\n \t\t\t\t    updates[i]->refname);\n \t\t\treturn 1;\n \t\t}\n+\t}\n \treturn 0;\n }\n \n@@ -3734,13 +3745,17 @@ int transaction_commit(struct 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 \t\tint flags = update->flags;\n \n+\t\tif (update->update_type != UPDATE_SHA1)\n+\t\t\tcontinue;\n+\n \t\tif (is_null_sha1(update->new_sha1))\n \t\t\tflags |= REF_DELETING;\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@@ -3762,6 +3777,8 @@ int transaction_commit(struct 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\tif (write_ref_sha1(update->lock, update->new_sha1,\n \t\t\t\t\t   update->msg)) {\n@@ -3779,6 +3796,8 @@ int transaction_commit(struct 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 = TRANSACTION_GENERIC_ERROR;\n-- \n2.2.0.rc3\n"},{"id":"252635","messageId":"1417066485-24921-4-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1417066485-24921-1-git-send-email-sbeller@google.com","subject":"[PATCH 3/4] refs.c: add a transaction function to append a reflog entry","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-27T05:34:44Z","receivedAt":"2014-11-27T05:34:44Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nDefine 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. We can pass a flag to this\nfunction, which can truncate the the reflog file before we write the\nupdate.\n\nWhen 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. This change only affects\nwhether or not a reflog entry should be generated and written. If msg == NULL\nthen no such entry will be written.\n\nOrthogonal to this we have a boolean flag REFLOG_TRUNCATE which is used to\ntell the transaction system to \"truncate the reflog and thus discard all\nprevious users\".\n\nAt the current time the only place where we use msg == NULL is also the\nplace, where we use REFLOG_TRUNCATE. Even though these two settings are\ncurrently only ever used together it still makes sense to have them through\ntwo separate knobs.\n\nThis allows future consumers of this API that may want to do things\ndifferently. For example someone can do:\n  msg=\"Reflog truncated by Bob because ...\" + REFLOG_TRUNCATE\nand have it truncate the log and have it start fresh with an initial message\nthat explains the log was truncated. This API allows that.\n\nDuring one transaction we allow to make multiple reflog updates to the\nsame ref. This means we only need to lock the reflog once, during the first\nupdate that 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 (internally in refs.c) such as:\n\nt = transaction_begin()\ntransaction_reflog_update(t, \"foo\", REFLOG_TRUNCATE, NULL);\nloop-over-something...\n   transaction_reflog_update(t, \"foo\", 0, <message>);\ntransaction_commit(t)\n\nwhere we first truncate the reflog and then build the new content one line\nat a time.\n\nWhile this technically looks like O(n2) behavior it is not that bad.\nWe only do this loop for transactions that cover a single ref during\nreflog expire. This means that the linear search inside\ntransaction_update_reflog() will find the match on the very first entry\nthus making it O(1) and not O(n) or our usecases. Thus the whole expire\nbecomes O(n) instead of O(n2). If in the future we start doing this for many\nrefs in one single transaction we might want to optimize this.\nBut there is no need to complexify the code and optimize for future usecases\nthat might never materialize at this stage.\n\nInstead of recording the line by line reflog updates in memory, use a\ntempfile: .git/tmp_reflog_XXXXXX which we write the entries to as the\ntransaction is built. Then just rename this file onto the destination\nreflog file when the transaction is committed.\n\ntodo:\nThis patch needs to add an atexit() thingy as well to ensure that\nany remaining files are unlinked on exit, just like the lock_file() thing does.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\nThis is a complete rewrite from previous series. Sorry no diff to previous versions.\n\n refs.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n refs.h |  21 ++++++++++\n 2 files changed, 167 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 84e086f..a1af703 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3517,7 +3517,8 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n }\n \n enum transaction_update_type {\n-\tUPDATE_SHA1 = 0\n+\tUPDATE_SHA1 = 0,\n+\tUPDATE_LOG = 1\n };\n \n /**\n@@ -3530,11 +3531,18 @@ 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;  /* The flags to transaction_update_ref[log] are defined\n+\t\t     * in refs.h\n+\t\t     */\n \tint have_old; /* 1 if old_sha1 is valid, 0 otherwise */\n \tstruct ref_lock *lock;\n \tint type;\n \tchar *msg;\n+\n+\t/* used by reflog updates */\n+\tchar *tmp_reflog;\n+\tint reflog_fd;\n+\n \tconst char refname[FLEX_ARRAY];\n };\n \n@@ -3580,6 +3588,7 @@ void transaction_free(struct transaction *transaction)\n \t\treturn;\n \n \tfor (i = 0; i < transaction->nr; i++) {\n+\t\tfree(transaction->updates[i]->tmp_reflog);\n \t\tfree(transaction->updates[i]->msg);\n \t\tfree(transaction->updates[i]);\n \t}\n@@ -3601,6 +3610,116 @@ static struct ref_update *add_update(struct transaction *transaction,\n \treturn update;\n }\n \n+int transaction_update_reflog(struct 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 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+\tstruct strbuf buf = STRBUF_INIT;\n+\tint i;\n+\n+\tif (transaction->state != TRANSACTION_OPEN)\n+\t\tdie(\"BUG: update_reflog called for transaction that is not open\");\n+\n+\t/* Check if there is another reflog update for this ref already. */\n+\tfor (i = 0; transaction->nr > 0 && i < transaction->nr; 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    refname)) {\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\t/* When starting the transaction or when we did not find the ref,\n+\t * we will need to create a new temporary file. */\n+\tif (transaction->nr == 0 || i == transaction->nr) {\n+\t\tint orig_fd;\n+\t\tupdate = add_update(transaction, refname, UPDATE_LOG);\n+\n+\t\torig_fd = open(git_path(\"logs/%s\", refname), O_RDONLY);\n+\t\tif (orig_fd < 0) {\n+\t\t\tconst char *str = \"Cannot open reflog for '%s'. %s\";\n+\n+\t\t\tstrbuf_addf(err, str, refname, strerror(errno));\n+\t\t\ttransaction->state = TRANSACTION_CLOSED;\n+\t\t\treturn 1;\n+\t\t}\n+\n+\t\tupdate->tmp_reflog = xstrdup(git_path(\".tmp_reflog_XXXXXX\"));\n+\t\tupdate->reflog_fd = mkstemp(update->tmp_reflog);\n+\t\tif (update->reflog_fd == -1) {\n+\t\t\tconst char *str = \"Could not create temporary \"\n+\t\t\t  \"reflog for '%s'. %s\";\n+\n+\t\t\tclose(orig_fd);\n+\t\t\tstrbuf_addf(err, str, refname, strerror(errno));\n+\t\t\ttransaction->state = TRANSACTION_CLOSED;\n+\t\t\treturn 1;\n+\t\t}\n+\t\tif (adjust_shared_perm(update->tmp_reflog)) {\n+\t\t\tstrbuf_addf(err, \"Could not fix permission bits for \"\n+\t\t\t\t    \"reflog: %s. %s\",\n+\t\t\t\t    update->tmp_reflog, strerror(errno));\n+\t\t\tclose(orig_fd);\n+\t\t\tunlink_or_warn(update->tmp_reflog);\n+\t\t\tclose(update->reflog_fd);\n+\t\t\tupdate->reflog_fd = -1;\n+\t\t\ttransaction->state = TRANSACTION_CLOSED;\n+\t\t\treturn 1;\n+\t\t}\n+\t\tif (copy_fd(orig_fd, update->reflog_fd)) {\n+\t\t\tstrbuf_addf(err, \"Could not copy reflog: %s. %s\",\n+\t\t\t\t    refname, strerror(errno));\n+\t\t\tclose(orig_fd);\n+\t\t\tunlink_or_warn(update->tmp_reflog);\n+\t\t\tclose(update->reflog_fd);\n+\t\t\tupdate->reflog_fd = -1;\n+\t\t\ttransaction->state = TRANSACTION_CLOSED;\n+\t\t\treturn 1;\n+\t\t}\n+\t\tclose(orig_fd);\n+\t} else {\n+\t\tupdate = transaction->updates[i];\n+\t}\n+\n+\tif (flags & REFLOG_TRUNCATE) {\n+\t\tif (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||\n+\t\t\tftruncate(update->reflog_fd, 0)) {\n+\t\t\tstrbuf_addf(err, \"Could not truncate reflog: %s. %s\",\n+\t\t\t\t    refname, strerror(errno));\n+\t\t\tunlink_or_warn(update->tmp_reflog);\n+\t\t\tclose(update->reflog_fd);\n+\t\t\tupdate->reflog_fd = -1;\n+\t\t\ttransaction->state = TRANSACTION_CLOSED;\n+\t\t\treturn 1;\n+\t\t}\n+\t}\n+\tif (email)\n+\t\tstrbuf_addf(&buf, \"%s %lu %+05d\", email, timestamp, tz);\n+\n+\tif (msg &&\n+\t    log_ref_write_fd(update->reflog_fd,\n+\t\t\t     old_sha1, new_sha1,\n+\t\t\t     buf.buf, msg)) {\n+\t\tstrbuf_addf(err, \"Could not write to reflog: %s. %s\",\n+\t\t\t    refname, strerror(errno));\n+\t\tunlink_or_warn(update->tmp_reflog);\n+\t\tclose(update->reflog_fd);\n+\t\tupdate->reflog_fd = -1;\n+\t\ttransaction->state = TRANSACTION_CLOSED;\n+\t\tstrbuf_release(&buf);\n+\t\treturn 1;\n+\t}\n+\tstrbuf_release(&buf);\n+\n+\treturn 0;\n+}\n+\n int transaction_update_ref(struct transaction *transaction,\n \t\t\t   const char *refname,\n \t\t\t   const unsigned char *new_sha1,\n@@ -3815,6 +3934,31 @@ int transaction_commit(struct transaction *transaction,\n \t}\n \tfor (i = 0; i < delnum; i++)\n \t\tunlink_or_warn(git_path(\"logs/%s\", delnames[i]));\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 (close(update->reflog_fd) == -1) {\n+\t\t\terror(\"Could not commit temporary reflog: %s. %s\",\n+\t\t\t      update->refname, strerror(errno));\n+\t\t\tupdate->reflog_fd = -1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tupdate->reflog_fd = -1;\n+\t\tif (rename(update->tmp_reflog,\n+\t\t\t   git_path(\"logs/%s\", update->refname))) {\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\tcontinue;\n+\t\t}\n+\t}\n+\n \tclear_loose_ref_cache(&ref_cache);\n \n cleanup:\ndiff --git a/refs.h b/refs.h\nindex 556adfd..9f70b89 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -328,6 +328,27 @@ int transaction_delete_ref(struct transaction *transaction,\n \t\t\t   struct strbuf *err);\n \n /*\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+ * If msg is NULL no update will be written to the log.\n+ */\n+int transaction_update_reflog(struct 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 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.\n  *\n-- \n2.2.0.rc3\n"},{"id":"252633","messageId":"1417066485-24921-5-git-send-email-sbeller@google.com","threadId":"37991","inReplyTo":"1417066485-24921-1-git-send-email-sbeller@google.com","subject":"[PATCH 4/4] reflog.c: use a reflog transaction when writing during expire","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-27T05:34:45Z","receivedAt":"2014-11-27T05:34:45Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Ronnie Sahlberg <sahlberg@google.com>\n\nUse a transaction for all updates during expire_reflog.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/reflog.c | 85 ++++++++++++++++++++++++--------------------------------\n 1 file changed, 37 insertions(+), 48 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 2d85d26..6bb7454 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 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,20 @@ 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\t\tlock->lk->filename.buf);\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_ref(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\tgoto cleanup;\n \t\t}\n+\t\tif (transaction_commit(cb.t, &err))\n+\t\t\tstatus |= error(\"%s\", err.buf);\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 \n-- \n2.2.0.rc3\n"}]}