{"thread":{"id":"38126","subject":"[PATCH 0/8] Making reflog modifications part of the transactions API","startedAt":"2014-12-06T02:46:27Z","lastAt":"2014-12-14T23:17:05Z","messageCount":18,"participants":["Stefan Beller","Jonathan Nieder","Junio C Hamano","Michael Haggerty","ronnie sahlberg"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"253218","messageId":"1417833995-25687-1-git-send-email-sbeller@google.com","threadId":"38126","inReplyTo":null,"subject":"[PATCH 0/8] Making reflog modifications part of the transactions API","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-06T02:46:27Z","receivedAt":"2014-12-06T02:46:27Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This goes on top of Michaels series. The idea of this series is make the\nreflogs being part of the transaction API, so it will be part of the contract\nof transaction_commit to either commit all the changes or none at all.\n\nCurrently when using the transaction API to change refs, also reflogs are changed.\nBut the changes to the reflogs just happen as a side effect and not as part of\nthe atomic part of changes we want to commit altogether.\n\nRonnie Sahlberg (3):\n  refs.c: use a reflog transaction when writing during expire\n  refs.c: rename log_ref_setup to create_reflog\n  refs.c: allow deleting refs with a broken sha1\n\nStefan Beller (5):\n  refs.c: let fprintf handle the formatting\n  refs.c: rename the transaction functions\n  refs.c: rename transaction.updates to transaction.ref_updates\n  refs.c: add transaction function to append to the reflog\n  refs.c: add transaction function to delete the reflog\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/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                 | 359 ++++++++++++++++++++++++++++++++++---------------\n refs.h                 |  43 +++---\n sequencer.c            |  12 +-\n t/t3200-branch.sh      |   8 ++\n walker.c               |  10 +-\n 16 files changed, 359 insertions(+), 209 deletions(-)\n\n-- \n2.2.0\n"},{"id":"253219","messageId":"1417833995-25687-2-git-send-email-sbeller@google.com","threadId":"38126","inReplyTo":"1417833995-25687-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/8] refs.c: let fprintf handle the formatting","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-06T02:46:28Z","receivedAt":"2014-12-06T02:46:28Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Instead of calculating, whether to put a plus or minus sign, offload\nthe responsibilty to the fprintf function.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\nNotes:\n    This patch was already sent as a single patch to the list,\n    but not yet acknowledge in any way, so I am including it here.\n    \n    This series goes on top of Michaels series\n    https://github.com/mhagger/git/tree/reflog-expire-api-v1\n\n refs.c | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 40c5591..1147216 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3972,12 +3972,9 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \t\t\tprintf(\"prune %s\", message);\n \t} else {\n \t\tif (cb->newlog) {\n-\t\t\tchar sign = (tz < 0) ? '-' : '+';\n-\t\t\tint zone = (tz < 0) ? (-tz) : tz;\n-\t\t\tfprintf(cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n+\t\t\tfprintf(cb->newlog, \"%s %s %s %lu %+05d\\t%s\",\n \t\t\t\tsha1_to_hex(osha1), sha1_to_hex(nsha1),\n-\t\t\t\temail, timestamp, sign, zone,\n-\t\t\t\tmessage);\n+\t\t\t\temail, timestamp, tz, message);\n \t\t\thashcpy(cb->last_kept_sha1, nsha1);\n \t\t}\n \t\tif (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n-- \n2.2.0\n"},{"id":"253225","messageId":"1417833995-25687-3-git-send-email-sbeller@google.com","threadId":"38126","inReplyTo":"1417833995-25687-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/8] refs.c: rename the transaction functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-06T02:46:29Z","receivedAt":"2014-12-06T02:46:29Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Rename 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\nThe goal in the long term is to have different things running through\nthe transaction api, such as the .git/config file. The dream is to\nhave any possible modification passing the transaction api.\nEven if we don't achieve this goal, this change still makes sense\nas it's slightly easier to read without the ref_ prefixes.\n\n[sb: cherry-picked on top of mhagger/reflog-expire-api-v1]\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n\nNotes:\n    no changes since last sending it.\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                 | 35 +++++++++++-----------\n sequencer.c            | 12 ++++----\n walker.c               | 10 +++----\n 12 files changed, 126 insertions(+), 125 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 1147216..9f2628b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -35,7 +35,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@@ -2545,23 +2545,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@@ -2723,20 +2723,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@@ -3545,9 +3545,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@@ -3555,21 +3555,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@@ -3584,7 +3584,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@@ -3596,7 +3596,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@@ -3607,7 +3607,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@@ -3631,23 +3631,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@@ -3655,17 +3655,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@@ -3680,7 +3680,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@@ -3708,8 +3708,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@@ -3718,11 +3718,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@@ -3801,7 +3801,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 2957641..40607d6 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -2,22 +2,22 @@\n #define REFS_H\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@@ -33,7 +33,7 @@\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@@ -172,7 +172,8 @@ extern int is_branch(const char *refname);\n extern int peel_ref(const char *refname, unsigned char *sha1);\n \n /*\n- * Flags controlling ref_transaction_update(), 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@@ -244,13 +245,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@@ -266,7 +267,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@@ -282,7 +283,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@@ -296,7 +297,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@@ -312,13 +313,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\n"},{"id":"253226","messageId":"1417833995-25687-4-git-send-email-sbeller@google.com","threadId":"38126","inReplyTo":"1417833995-25687-1-git-send-email-sbeller@google.com","subject":"[PATCH 3/8] refs.c: rename transaction.updates to transaction.ref_updates","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-06T02:46:30Z","receivedAt":"2014-12-06T02:46:30Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The updates are only holding refs not reflogs, so express it to the reader.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n\nNotes:\n    no changes since last sending it.\n\n refs.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 9f2628b..aad44d5 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3556,7 +3556,7 @@ enum transaction_state {\n  * as atomically as possible.  This structure is opaque to callers.\n  */\n struct transaction {\n-\tstruct ref_update **updates;\n+\tstruct ref_update **ref_updates;\n \tsize_t alloc;\n \tsize_t nr;\n \tenum transaction_state state;\n@@ -3577,10 +3577,10 @@ void transaction_free(struct transaction *transaction)\n \t\treturn;\n \n \tfor (i = 0; i < transaction->nr; i++) {\n-\t\tfree(transaction->updates[i]->msg);\n-\t\tfree(transaction->updates[i]);\n+\t\tfree(transaction->ref_updates[i]->msg);\n+\t\tfree(transaction->ref_updates[i]);\n \t}\n-\tfree(transaction->updates);\n+\tfree(transaction->ref_updates);\n \tfree(transaction);\n }\n \n@@ -3591,8 +3591,8 @@ static struct ref_update *add_update(struct transaction *transaction,\n \tstruct ref_update *update = xcalloc(1, sizeof(*update) + len + 1);\n \n \tstrcpy((char *)update->refname, refname);\n-\tALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);\n-\ttransaction->updates[transaction->nr++] = update;\n+\tALLOC_GROW(transaction->ref_updates, transaction->nr + 1, transaction->alloc);\n+\ttransaction->ref_updates[transaction->nr++] = update;\n \treturn update;\n }\n \n@@ -3714,7 +3714,7 @@ int transaction_commit(struct transaction *transaction,\n \tint ret = 0, delnum = 0, i;\n \tconst char **delnames;\n \tint n = transaction->nr;\n-\tstruct ref_update **updates = transaction->updates;\n+\tstruct ref_update **updates = transaction->ref_updates;\n \n \tassert(err);\n \n-- \n2.2.0\n"},{"id":"253220","messageId":"1417833995-25687-5-git-send-email-sbeller@google.com","threadId":"38126","inReplyTo":"1417833995-25687-1-git-send-email-sbeller@google.com","subject":"[PATCH 4/8] refs.c: add transaction function to append to the reflog","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-06T02:46:31Z","receivedAt":"2014-12-06T02:46:31Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Currently a transaction can include one or more reflog updates. But these\nare not part of the contract of the transaction committing all or nothing.\n\nIntroduce a function transaction_update_reflog, which adds a reflog update\nto a transaction. Later patches will update code that writes to reflogs to\nuse this function.\n\nThis will allow us to write code such as:\n\n\tt = transaction_begin()\n\ttransaction_truncate_reflog(t, \"foo\"); // introduced by next patch\n\tloop-over-something...\n\t\tif (want_reflog_entry(...))\n\t\t\ttransaction_reflog_update(t, \"foo\", 0, <message>);\n\ttransaction_commit(t)\n\ntransaction_ref_update still updates the reflog along with its ref update.\nA later patch will make it use transaction_update_reflog internally.\n\nUnlike transaction_update_ref, this writes out the proposed contents of the\nreflog to a temporary file at transaction_reflog_update time instead of\nwaiting for the transaction waiting to be committed. This avoids an\nexplosion of memory usage when writing lots of reflog updates within a\nsingle transaction.\n\nThis requires changing where the temporary file is located. If the temporary\nfile is located at $GIT_DIR/logs/$REF_NAME.lock, then a transaction that\ntries to remove refs/heads/foo and introduce refs/heads/foo/bar would run\ninto a directory/file conflict when trying to write to\n$GIT_DIR/logs/refs/heads/foo/bar.lock because the reflog for branch foo is\nnot safe to remove yet.\n\nWe work around this by placing the temporary files at\n$GIT_DIR/logs/lock/$REF_NAME.lock Putting the temporary file under .git/logs\nensures it's likely that it will be in the same file system as the reflogs\nand can be atomically renamed into place.\n\nDuring one transaction we allow to make multiple reflog updates to the same\nref. This 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\ntransaction_update_reflog should lock the corresponding ref to avoid having\nto compete with other reflog updates, but this will be deferred to a later\npatch for simplicity.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\nNotes:\n    This has been sent to the list before, but now it is split up\n    into this and the next patch.\n    The commit message was overhauled, a huge thanks to Jonathan!\n    \n    There are no major changes in the code.\n\n refs.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 92 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex aad44d5..d767418 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3559,19 +3559,30 @@ struct transaction {\n \tstruct ref_update **ref_updates;\n \tsize_t alloc;\n \tsize_t nr;\n+\n+\t/*\n+\t * Sorted list of reflogs to be committed.\n+\t * the util points to the lock_file.\n+\t */\n+\tstruct string_list reflog_updates;\n \tenum transaction_state state;\n };\n \n struct transaction *transaction_begin(struct strbuf *err)\n {\n+\tstruct transaction *ret;\n+\n \tassert(err);\n \n-\treturn xcalloc(1, sizeof(struct transaction));\n+\tret = xcalloc(1, sizeof(struct transaction));\n+\tret->reflog_updates.strdup_strings = 1;\n+\treturn ret;\n }\n \n void transaction_free(struct transaction *transaction)\n {\n \tint i;\n+\tstruct string_list_item *item;\n \n \tif (!transaction)\n \t\treturn;\n@@ -3581,6 +3592,12 @@ void transaction_free(struct transaction *transaction)\n \t\tfree(transaction->ref_updates[i]);\n \t}\n \tfree(transaction->ref_updates);\n+\n+\tfor_each_string_list_item(item, &transaction->reflog_updates) {\n+\t\trollback_lock_file(item->util);\n+\t}\n+\tstring_list_clear(&transaction->reflog_updates, 0);\n+\n \tfree(transaction);\n }\n \n@@ -3631,6 +3648,71 @@ int transaction_update_ref(struct transaction *transaction,\n \treturn 0;\n }\n \n+/*\n+ * Append a reflog entry for refname.\n+ */\n+static int transaction_update_reflog(struct transaction *transaction,\n+\t\t\t\t     const char *refname,\n+\t\t\t\t     const unsigned char *new_sha1,\n+\t\t\t\t     const unsigned char *old_sha1,\n+\t\t\t\t     const char *email,\n+\t\t\t\t     unsigned long timestamp, int tz,\n+\t\t\t\t     const char *msg,\n+\t\t\t\t     struct strbuf *err)\n+{\n+\tstruct lock_file *lock;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct string_list_item *item;\n+\n+\tif (transaction->state != TRANSACTION_OPEN)\n+\t\tdie(\"BUG: update_reflog called for transaction that is not open\");\n+\n+\titem = string_list_insert(&transaction->reflog_updates, refname);\n+\tif (!item->util) {\n+\t\tint infd;\n+\t\tchar *path = git_path(\"logs/locks/%s\", refname);\n+\t\tlock = xcalloc(1, sizeof(struct lock_file));\n+\t\titem->util = lock;\n+\t\tif (safe_create_leading_directories(path)) {\n+\t\t\tstrbuf_addf(err, \"could not create leading directories of '%s': %s\",\n+\t\t\t\t    path, strerror(errno));\n+\t\t\tgoto failure;\n+\t\t}\n+\n+\t\tif (hold_lock_file_for_update(lock, path, 0) < 0) {\n+\t\t\tunable_to_lock_message(path, errno, err);\n+\t\t\tgoto failure;\n+\t\t}\n+\n+\t\t/* copy existing reflog over */\n+\t\tif ((infd = open(git_path(\"logs/%s\", refname), O_RDONLY)) < 0) {\n+\t\t\tstrbuf_addf(err, \"could not open the reflog '%s': %s\",\n+\t\t\t\t    path, strerror(errno));\n+\t\t\tgoto failure;\n+\t\t}\n+\t\tif (copy_fd(infd, lock->fd))\n+\t\t\tgoto failure;\n+\t\tclose(infd);\n+\t}\n+\tlock = item->util;\n+\n+\tif (email)\n+\t\tstrbuf_addf(&buf, \"%s %lu %+05d\", email, timestamp, tz);\n+\n+\tif (log_ref_write_fd(lock->fd, old_sha1, new_sha1, buf.buf, msg)) {\n+\t\tstrbuf_addf(err, \"Could not write to reflog: '%s': %s\",\n+\t\t\t    refname, strerror(errno));\n+\t\tgoto failure;\n+\t}\n+\n+\tstrbuf_release(&buf);\n+\treturn 0;\n+failure:\n+\tstrbuf_release(&buf);\n+\ttransaction->state = TRANSACTION_CLOSED;\n+\treturn -1;\n+}\n+\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@@ -3715,13 +3797,14 @@ int transaction_commit(struct transaction *transaction,\n \tconst char **delnames;\n \tint n = transaction->nr;\n \tstruct ref_update **updates = transaction->ref_updates;\n+\tstruct string_list_item *item;\n \n \tassert(err);\n \n \tif (transaction->state != TRANSACTION_OPEN)\n \t\tdie(\"BUG: commit called for transaction that is not open\");\n \n-\tif (!n) {\n+\tif (!n && !transaction->reflog_updates.nr) {\n \t\ttransaction->state = TRANSACTION_CLOSED;\n \t\treturn 0;\n \t}\n@@ -3798,6 +3881,13 @@ 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 updates*/\n+\tfor_each_string_list_item(item, &transaction->reflog_updates) {\n+\t\tstruct lock_file *lock = item->util;\n+\t\tcommit_lock_file_to(lock, git_path(\"logs/%s\", item->string));\n+\t}\n+\n \tclear_loose_ref_cache(&ref_cache);\n \n cleanup:\n-- \n2.2.0\n"},{"id":"253222","messageId":"1417833995-25687-6-git-send-email-sbeller@google.com","threadId":"38126","inReplyTo":"1417833995-25687-1-git-send-email-sbeller@google.com","subject":"[PATCH 5/8] refs.c: add transaction function to delete the reflog","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-06T02:46:32Z","receivedAt":"2014-12-06T02:46:32Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This continues the work of the previous patch as reflogs not\nonly grow, but also need a cut sometimes. This patch introduces\ntransaction_delete_reflog as part of the transaction API to\ndelete the reflog.\n\nThis function serves two purposes. It can be used to actually\ndelete the reflog as the name indicates. The other purpose is\ntruncation of the reflog and rewriting it.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n refs.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 61 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex d767418..57f4941 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3649,6 +3649,56 @@ int transaction_update_ref(struct transaction *transaction,\n }\n \n /*\n+ * Delete the reflog for the given refname.\n+ *\n+ */\n+static int transaction_delete_reflog(struct transaction *transaction,\n+\t\t\t\t       const char *refname,\n+\t\t\t\t       struct strbuf *err)\n+{\n+\tstruct lock_file *lock;\n+\tstruct string_list_item *item;\n+\n+\tif (transaction->state != TRANSACTION_OPEN)\n+\t\tdie(\"BUG: delete_reflog called for transaction that is not open\");\n+\n+\titem = string_list_insert(&transaction->reflog_updates, refname);\n+\n+\tif (!item->util) {\n+\t\tchar *path = git_path(\"logs/locks/%s\", refname);\n+\t\tlock = xcalloc(1, sizeof(struct lock_file));\n+\t\titem->util = lock;\n+\t\tif (safe_create_leading_directories(path)) {\n+\t\t\tstrbuf_addf(err, \"could not create leading directories of '%s': %s\",\n+\t\t\t\t    path, strerror(errno));\n+\t\t\tgoto failure;\n+\t\t}\n+\n+\t\tif (hold_lock_file_for_update(lock, path, 0) < 0) {\n+\t\t\tunable_to_lock_message(path, errno, err);\n+\t\t\tgoto failure;\n+\t\t}\n+\t\t/* The empty file indicates transaction_commit to\n+\t\t * delete the reflog */\n+\t\treturn 0;\n+\t}\n+\n+\t/* The transaction already writes to this reflog.  Clear it. */\n+\tlock = item->util;\n+\tif (lseek(lock->fd, 0, SEEK_SET) < 0 ||\n+\t    ftruncate(lock->fd, 0)) {\n+\t\tstrbuf_addf(err, \"cannot truncate reflog '%s': %s\",\n+\t\t\t    refname, strerror(errno));\n+\t\tgoto failure;\n+\t}\n+\treturn 0;\n+\n+failure:\n+\ttransaction->state = TRANSACTION_CLOSED;\n+\treturn -1;\n+}\n+\n+/*\n  * Append a reflog entry for refname.\n  */\n static int transaction_update_reflog(struct transaction *transaction,\n@@ -3885,7 +3935,17 @@ int transaction_commit(struct transaction *transaction,\n \t/* Commit all reflog updates*/\n \tfor_each_string_list_item(item, &transaction->reflog_updates) {\n \t\tstruct lock_file *lock = item->util;\n-\t\tcommit_lock_file_to(lock, git_path(\"logs/%s\", item->string));\n+\n+\t\t/* If the lock file is empty we want to delete the reflog*/\n+\t\toff_t filepos = lseek(lock->fd, 0, SEEK_END);\n+\t\tif (filepos < 0) {\n+\t\t\tret = TRANSACTION_GENERIC_ERROR;\n+\t\t\tgoto cleanup;\n+\t\t}\n+\t\tif (filepos)\n+\t\t\tcommit_lock_file_to(lock, git_path(\"logs/%s\", item->string));\n+\t\telse\n+\t\t\tremove_path(git_path(\"logs/%s\", item->string));\n \t}\n \n \tclear_loose_ref_cache(&ref_cache);\n-- \n2.2.0\n"},{"id":"253224","messageId":"1417833995-25687-7-git-send-email-sbeller@google.com","threadId":"38126","inReplyTo":"1417833995-25687-1-git-send-email-sbeller@google.com","subject":"[PATCH 6/8] refs.c: use a reflog transaction when writing during expire","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-06T02:46:33Z","receivedAt":"2014-12-06T02:46:33Z","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\n[sb: This was once a patch series on its own. I cherry-picked these\npatches on top of Michaels series to cleanup the refs api. So any\nanomalies and bugs may be introduced by me.]\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\nNotes:\n    Maybe we can leave out the first patch of the series\n    as this one deletes the changes made in the first patch of the series.\n    \n    Originally authored by Ronnie for the reflogs.c file,\n    cherrypicked over to the refs.c file by Stefan\n\n refs.c | 85 +++++++++++++++++++++++++++---------------------------------------\n 1 file changed, 35 insertions(+), 50 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 57f4941..295ea09 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -4100,8 +4100,10 @@ struct expire_reflog_cb {\n \tunsigned int flags;\n \treflog_expiry_select_fn *select_fn;\n \tvoid *policy_cb;\n-\tFILE *newlog;\n+\tstruct transaction *t;\n+\tconst char *refname;\n \tunsigned char last_kept_sha1[20];\n+\tstruct strbuf *err;\n };\n \n static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n@@ -4116,15 +4118,18 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \n \tif ((*cb->select_fn)(osha1, nsha1, email, timestamp, tz,\n \t\t\t     message, policy_cb)) {\n-\t\tif (!cb->newlog)\n+\t\tif (!cb->t)\n \t\t\tprintf(\"would prune %s\", message);\n \t\telse if (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(\"prune %s\", message);\n \t} else {\n-\t\tif (cb->newlog) {\n-\t\t\tfprintf(cb->newlog, \"%s %s %s %lu %+05d\\t%s\",\n-\t\t\t\tsha1_to_hex(osha1), sha1_to_hex(nsha1),\n-\t\t\t\temail, timestamp, tz, message);\n+\t\tif (cb->t) {\n+\t\t\tif (transaction_update_reflog(cb->t, cb->refname,\n+\t\t\t\t\t\t      nsha1, osha1,\n+\t\t\t\t\t\t      email, timestamp,\n+\t\t\t\t\t\t      tz, message,\n+\t\t\t\t\t\t      cb->err))\n+\t\t\t\treturn -1;\n \t\t\thashcpy(cb->last_kept_sha1, nsha1);\n \t\t}\n \t\tif (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n@@ -4133,8 +4138,6 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \treturn 0;\n }\n \n-static struct lock_file reflog_lock;\n-\n extern int reflog_expire(const char *refname, const unsigned char *sha1,\n \t\t\t unsigned int flags,\n \t\t\t reflog_expiry_prepare_fn prepare_fn,\n@@ -4143,66 +4146,48 @@ extern int reflog_expire(const char *refname, const unsigned char *sha1,\n \t\t\t void *policy_cb_data)\n {\n \tstruct expire_reflog_cb cb;\n-\tstruct ref_lock *lock;\n-\tchar *log_file;\n \tint status = 0;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tmemset(&cb, 0, sizeof(cb));\n \tcb.flags = flags;\n \tcb.policy_cb = policy_cb_data;\n \tcb.select_fn = select_fn;\n+\tcb.err = &err;\n+\tcb.refname = refname;\n \n \t/*\n-\t * we take the lock for the ref itself to prevent it from\n-\t * getting updated.\n+\t * todo: we need to take the lock for the ref itself to\n+\t * prevent it from getting updated.\n \t */\n-\tlock = lock_ref_sha1_basic(refname, sha1, NULL, 0, NULL);\n-\tif (!lock)\n-\t\treturn error(\"cannot lock ref '%s'\", refname);\n-\tif (!reflog_exists(refname)) {\n-\t\tunlock_ref(lock);\n-\t\treturn 0;\n+\tcb.t = transaction_begin(&err);\n+\tif (!cb.t) {\n+\t\tstatus |= error(\"%s\", err.buf);\n+\t\tgoto cleanup;\n \t}\n-\n-\tlog_file = git_pathdup(\"logs/%s\", refname);\n-\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n-\t\tif (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0)\n-\t\t\tgoto failure;\n-\t\tcb.newlog = fdopen_lock_file(&reflog_lock, \"w\");\n-\t\tif (!cb.newlog)\n-\t\t\tgoto failure;\n+\tif (transaction_delete_reflog(cb.t, cb.refname, &err)) {\n+\t\tstatus |= error(\"%s\", err.buf);\n+\t\tgoto cleanup;\n \t}\n \n \t(*prepare_fn)(refname, sha1, cb.policy_cb);\n \tfor_each_reflog_ent(refname, expire_reflog_ent, &cb);\n \t(*cleanup_fn)(cb.policy_cb);\n \n+\n \tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n-\t\tif (close_lock_file(&reflog_lock)) {\n-\t\t\tstatus |= error(\"Couldn't write %s: %s\", log_file,\n-\t\t\t\t\tstrerror(errno));\n-\t\t} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\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\trollback_lock_file(&reflog_lock);\n-\t\t} else if (commit_lock_file(&reflog_lock)) {\n-\t\t\tstatus |= error(\"cannot rename %s.lock to %s\",\n-\t\t\t\t\tlog_file, log_file);\n-\t\t} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) && commit_ref(lock)) {\n-\t\t\tstatus |= error(\"Couldn't set %s\", lock->ref_name);\n+\t\tif ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\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(log_file);\n-\tunlock_ref(lock);\n+cleanup:\n+\ttransaction_free(cb.t);\n+\tstrbuf_release(&err);\n \treturn status;\n-\n- failure:\n-\trollback_lock_file(&reflog_lock);\n-\tfree(log_file);\n-\tunlock_ref(lock);\n-\treturn -1;\n }\n-- \n2.2.0\n"},{"id":"253223","messageId":"1417833995-25687-8-git-send-email-sbeller@google.com","threadId":"38126","inReplyTo":"1417833995-25687-1-git-send-email-sbeller@google.com","subject":"[PATCH 7/8] refs.c: rename log_ref_setup to create_reflog","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-06T02:46:34Z","receivedAt":"2014-12-06T02:46:34Z","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: Stefan Beller <sbeller@google.com>\n---\n\nNotes:\n    This is also just carried along from sending it last time.\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 295ea09..2effd66 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2943,16 +2943,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@@ -3021,16 +3021,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 40607d6..7599f81 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -183,11 +183,6 @@ extern int peel_ref(const char *refname, unsigned char *sha1);\n #define REF_NODEREF\t0x01\n #define REF_DELETING\t0x02\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@@ -197,6 +192,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\n"},{"id":"253221","messageId":"1417833995-25687-9-git-send-email-sbeller@google.com","threadId":"38126","inReplyTo":"1417833995-25687-1-git-send-email-sbeller@google.com","subject":"[PATCH 8/8] refs.c: allow deleting refs with a broken sha1","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-06T02:46:35Z","receivedAt":"2014-12-06T02:46:35Z","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: Stefan Beller <sbeller@google.com>\n---\n\nNotes:\n    Changes to previously sent version:\n     * do not introduce yet another flag, but stick with REF_DELETING\n       which was meant for this purpose.\n\n builtin/branch.c  | 5 +++--\n cache.h           | 7 +++++++\n refs.c            | 5 +++++\n t/t3200-branch.sh | 8 ++++++++\n 4 files changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 3b79c50..5fe1cac 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_DELETING)) {\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 2effd66..94b766f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1587,6 +1587,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,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t\tresolve_flags |= RESOLVE_REF_READING;\n \tif (flags & REF_DELETING) {\n \t\tresolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME;\n+\t\tresolve_flags |= RESOLVE_REF_ALLOW_BAD_SHA1;\n \t\tif (flags & REF_NODEREF)\n \t\t\tresolve_flags |= RESOLVE_REF_NO_RECURSE;\n \t}\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 432921b..1ea0d2c 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -163,6 +163,14 @@ test_expect_success 'git branch --list -d t should fail' '\n \ttest_path_is_missing .git/refs/heads/t\n '\n \n+test_expect_success 'git branch -d can delete ref with broken sha1' '\n+\techo \"pointing nowhere\" >.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 brokensha1 output\n+'\n+\n test_expect_success 'git branch --column' '\n \tCOLUMNS=81 git branch --column=column >actual &&\n \tcat >expected <<\\EOF &&\n-- \n2.2.0\n"},{"id":"253372","messageId":"CAGZ79kb1yiSENO+uANdeOKtcSCZgZA3UXSdE2F5PBA49erh2Jw@mail.gmail.com","threadId":"38126","inReplyTo":"1417833995-25687-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH 0/8] Making reflog modifications part of the transactions API","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-08T20:05:26Z","receivedAt":"2014-12-08T20:05:26Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"So I reviewed and examined this series and run into some problems.\n\nHow do we view an empty reflog, i.e. an empty file at logs/refs/heads/master?\nI was told this is not a valid state for a reflog. However even the\ntest suite sometimes\nproduces an empty reflog files.\nBelow there is a patch, which highlights the empty reflog when running\nt1301.sh verbosely.\n\n\n---\n t/t1301-shared-repo.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\nindex de42d21..6a35fc0 100755\n--- a/t/t1301-shared-repo.sh\n+++ b/t/t1301-shared-repo.sh\n@@ -113,7 +113,9 @@ done\n\n test_expect_success POSIXPERM 'git reflog expire honors\ncore.sharedRepository' '\n  git config core.sharedRepository group &&\n- git reflog expire --all &&\n+ git reflog master &&\n+ git reflog expire --all --verbose &&\n+ git reflog master &&\n  actual=\"$(ls -l .git/logs/refs/heads/master)\" &&\n  case \"$actual\" in\n  -rw-rw-*)\n-- \n2.2.0\n"},{"id":"253376","messageId":"20141208215415.GS16345@google.com","threadId":"38126","inReplyTo":"CAGZ79kb1yiSENO+uANdeOKtcSCZgZA3UXSdE2F5PBA49erh2Jw@mail.gmail.com","subject":"Re: [PATCH 0/8] Making reflog modifications part of the transactions API","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-12-08T21:54:15Z","receivedAt":"2014-12-08T21:54:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nStefan Beller wrote:\n\n> How do we view an empty reflog, i.e. an empty file at logs/refs/heads/master?\n\nThat's a good question.  I'm a little concerned about what 'git reflog\nexpire --updateref' would do in this case.\n\nIt looks like the longstanding behavior is for 'git reflog expire' to\nexpire any entry that is old enough, even if that entry describes the\ncurrent value of the ref and the reflog becomes empty.  The empty\nreflog file is kind of like having no reflog at all, except it means\ngit should log updates to that ref even if core.logallrefupdates is\nfalse.\n\nThat suggests that two separate operations delete_reflog (for 'git\nreflog delete') and truncate_reflog (for 'git reflog expire') would be\nneeded.\n\nThanks for catching it.\nJonathan\n"},{"id":"253580","messageId":"xmqqbnn9eudc.fsf@gitster.dls.corp.google.com","threadId":"38126","inReplyTo":"1417833995-25687-3-git-send-email-sbeller@google.com","subject":"Re: [PATCH 2/8] refs.c: rename the transaction functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-11T21:42:23Z","receivedAt":"2014-12-11T21:42:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> Rename the transaction functions. Remove the leading ref_ from the\n> names and append _ref to the names for functions that create/delete/\n> update sha1 refs.\n>\n> The goal in the long term is to have different things running through\n> the transaction api, such as the .git/config file.\n>\n> The dream is to...\n\nI was trying to polish somebody else's topic that does not have\nanythning to do with refs/reflogs/transactions today, and had to\nsuffer from conflicts from the textual difference between\ntransaction_begin and ref_transaction_begin, which I feel is totally\nunnecessary at this moment.\n\nI'd really prefer to see this done long after all the dust from\n\"ref\" changes from various people have settled, especially given\nthat we are not doing anything for that \"dream\" right now.\n\nThanks.\n"},{"id":"253581","messageId":"CAGZ79kZVWzvneL=1_5Wp7ANw2A67pjZi08A6g5Y5YN670+RnXQ@mail.gmail.com","threadId":"38126","inReplyTo":"xmqqbnn9eudc.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/8] refs.c: rename the transaction functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-11T21:48:54Z","receivedAt":"2014-12-11T21:48:54Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Dec 11, 2014 at 1:42 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> Rename the transaction functions. Remove the leading ref_ from the\n>> names and append _ref to the names for functions that create/delete/\n>> update sha1 refs.\n>>\n>> The goal in the long term is to have different things running through\n>> the transaction api, such as the .git/config file.\n>>\n>> The dream is to...\n>\n> I was trying to polish somebody else's topic that does not have\n> anythning to do with refs/reflogs/transactions today, and had to\n> suffer from conflicts from the textual difference between\n> transaction_begin and ref_transaction_begin, which I feel is totally\n> unnecessary at this moment.\n>\n> I'd really prefer to see this done long after all the dust from\n> \"ref\" changes from various people have settled, especially given\n> that we are not doing anything for that \"dream\" right now.\n>\n> Thanks.\n\nGood point. For the last days I was trying to write less invasive\npatches as I guessed this may become a problem.\n\nFeel free to drop these patches,\nI'll resend them later if they still make sense.\n"},{"id":"253582","messageId":"xmqq7fxxetzt.fsf@gitster.dls.corp.google.com","threadId":"38126","inReplyTo":"1417833995-25687-5-git-send-email-sbeller@google.com","subject":"Re: [PATCH 4/8] refs.c: add transaction function to append to the reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-11T21:50:30Z","receivedAt":"2014-12-11T21:50:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> Unlike transaction_update_ref, this writes out the proposed contents of the\n> reflog to a temporary file at transaction_reflog_update time instead of\n> waiting for the transaction waiting to be committed. This avoids an\n> explosion of memory usage when writing lots of reflog updates within a\n> single transaction.\n\nCopying an existing reflog with thousands of entries over so that I\ncan append a single new entry, just so that I can rollback by not\nrenaming?\n\nAfter ensuring that you are the only process that holds a write fd\nto the reflog file (e.g. by taking a lock on the ref itself),\nshouldn't you be able to ftell(), write() and then truncate() to\nroll back sanely before close()?  After all you are not protecting\nfrom power loss and other kinds of glitches that would leave *.lock\nfile behind, so...\n"},{"id":"253637","messageId":"548B150C.2090606@alum.mit.edu","threadId":"38126","inReplyTo":"1417833995-25687-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH 0/8] Making reflog modifications part of the transactions API","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-12T16:17:16Z","receivedAt":"2014-12-12T16:17:16Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 12/06/2014 03:46 AM, Stefan Beller wrote:\n> This goes on top of Michaels series. The idea of this series is make the\n> reflogs being part of the transaction API, so it will be part of the contract\n> of transaction_commit to either commit all the changes or none at all.\n> \n> Currently when using the transaction API to change refs, also reflogs are changed.\n> But the changes to the reflogs just happen as a side effect and not as part of\n> the atomic part of changes we want to commit altogether.\n\nWould you please explain why this patch series is still needed if my\n\"reflog_expire()\" series is accepted?\n\nreflog_expire() already has its own little transaction. Reflog\nexpiration never needs to be combined with other reference changes, so\nthere is no need for reflog expiration to be done via a ref_transaction.\n\nref_transaction_commit() already updates the reflog if necessary when a\nreference is updated. That update is part of the containing\nref_transaction, because it is locked by the lock on the reference\nitself at the beginning of the transaction and unlocked at the end of\nthe transaction [1]. It can't fail in normal circumstances because the\npreconditions for the transaction have already been checked.\n\nAs far as I can tell, the only thing left to do is provide a substitute\nfor log_ref_setup() a.k.a. create_reflog() that can be triggered within\na transaction. In my opinion the easiest way to do that is to give\nref_transaction_update()'s flag parameter an additional option,\nREF_CREATE_REFLOG, which forces the reference's reflog to be created if\nit does not already exist.\n\nWhat am I missing?\n\nMichael\n\n[1] The reflog updates are not atomic in the face of disk errors and\npower outages and stuff, but neither are reference updates. In other\nwords, I think reflog updates in ref_transaction_commit() are done as\nsafely as reference updates, which is surely good enough for this\nreference backend. Other reference backends can do a better job with\nboth while staying within the existing API.\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"253658","messageId":"CAGZ79kZB9TbygBM47Ug5i_FeR1c5w9btbayNEU3D2PeObCwzMw@mail.gmail.com","threadId":"38126","inReplyTo":"548B150C.2090606@alum.mit.edu","subject":"Re: [PATCH 0/8] Making reflog modifications part of the transactions API","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-12T20:51:44Z","receivedAt":"2014-12-12T20:51:44Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Dec 12, 2014 at 8:17 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> On 12/06/2014 03:46 AM, Stefan Beller wrote:\n>> This goes on top of Michaels series. The idea of this series is make the\n>> reflogs being part of the transaction API, so it will be part of the contract\n>> of transaction_commit to either commit all the changes or none at all.\n>>\n>> Currently when using the transaction API to change refs, also reflogs are changed.\n>> But the changes to the reflogs just happen as a side effect and not as part of\n>> the atomic part of changes we want to commit altogether.\n>\n> Would you please explain why this patch series is still needed if my\n> \"reflog_expire()\" series is accepted?\n\nIf we can live with the fact that reflog and refs have different APIs\nthen, we can probably\ndrop that series. That series may also have performance issues, Junio\npointed out.\nMost likely I'll look at the remaining 4 patch series from Ronnie, if\nI can put them on\ntop of your series now.\n\n>\n> reflog_expire() already has its own little transaction. Reflog\n> expiration never needs to be combined with other reference changes, so\n> there is no need for reflog expiration to be done via a ref_transaction.\n>\n> ref_transaction_commit() already updates the reflog if necessary when a\n> reference is updated. That update is part of the containing\n> ref_transaction, because it is locked by the lock on the reference\n> itself at the beginning of the transaction and unlocked at the end of\n> the transaction [1]. It can't fail in normal circumstances because the\n> preconditions for the transaction have already been checked.\n\n\n>\n> As far as I can tell, the only thing left to do is provide a substitute\n> for log_ref_setup() a.k.a. create_reflog() that can be triggered within\n> a transaction. In my opinion the easiest way to do that is to give\n> ref_transaction_update()'s flag parameter an additional option,\n> REF_CREATE_REFLOG, which forces the reference's reflog to be created if\n> it does not already exist.\n\nI already ported the patch which replaces log_ref_setup() by create_reflog()\non top of your series-v1, but I refrained to send it out as Junio\nseems to dislike\ninvasive patches[1] or possible merge conflicts so I was waiting for\ndiscussion on\nyour patch series calm down and be merged to next at least.\n\nApart from that rename, I also looked into flagging REF_CREATE_REFLOG\nadditionally.\nThough I run into problems there (test suite doesn't pass and I did\nnot find the mistake for 5 hours :/)\n\n>\n> What am I missing?\n\nI think the original plan of Ronnie was to have one and only one transaction\nAPI, which would be used for anything eventually (refs, reflogs,\nconfig, what have you)\nIf we go for a relaxed version of that, we'd be fine.\n\nThanks,\nStefan\n\n\n[1] http://www.spinics.net/lists/git/msg243451.html\n     The patch in question is not an invasive patch, but still touching places\n     which may be touched currently by your series. So I just wanted\nto wait a bit.\n"},{"id":"253660","messageId":"CAN05THRiMiQCqO1VZX7zQeWLBZMPG_sx1aVvdGirAoEh2Hd9Dw@mail.gmail.com","threadId":"38126","inReplyTo":"548B150C.2090606@alum.mit.edu","subject":"Re: [PATCH 0/8] Making reflog modifications part of the transactions API","fromName":"ronnie sahlberg","fromEmail":"ronniesahlberg@gmail.com","sentAt":"2014-12-12T21:16:07Z","receivedAt":"2014-12-12T21:16:07Z","isPatch":true,"sender":{"key":"ronniesahlberg@gmail.com","avatar":null},"body":"On Fri, Dec 12, 2014 at 11:17 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> On 12/06/2014 03:46 AM, Stefan Beller wrote:\n>> This goes on top of Michaels series. The idea of this series is make the\n>> reflogs being part of the transaction API, so it will be part of the contract\n>> of transaction_commit to either commit all the changes or none at all.\n>>\n>> Currently when using the transaction API to change refs, also reflogs are changed.\n>> But the changes to the reflogs just happen as a side effect and not as part of\n>> the atomic part of changes we want to commit altogether.\n>\n> Would you please explain why this patch series is still needed if my\n> \"reflog_expire()\" series is accepted?\n>\n> reflog_expire() already has its own little transaction. Reflog\n> expiration never needs to be combined with other reference changes, so\n> there is no need for reflog expiration to be done via a ref_transaction.\n>\n> ref_transaction_commit() already updates the reflog if necessary when a\n> reference is updated. That update is part of the containing\n> ref_transaction, because it is locked by the lock on the reference\n> itself at the beginning of the transaction and unlocked at the end of\n> the transaction [1]. It can't fail in normal circumstances because the\n> preconditions for the transaction have already been checked.\n>\n> As far as I can tell, the only thing left to do is provide a substitute\n> for log_ref_setup() a.k.a. create_reflog() that can be triggered within\n> a transaction. In my opinion the easiest way to do that is to give\n> ref_transaction_update()'s flag parameter an additional option,\n> REF_CREATE_REFLOG, which forces the reference's reflog to be created if\n> it does not already exist.\n>\n> What am I missing?\n>\n\nMy original idea was to clean up a bit of the reflog handling API and\nhave one single transaction API for all  ref and reflog operations.\n\nThink future use cases where you have a database backend for both refs\nand reflogs. It would be very nice to have a single atomic transaction\nthat would either commit or fail atomically any update to refs and/or\nreflogs.\nOtherwise you would have all consumers of the API have to invent their\nown transaction and rewind support : 'oh the ref transaction failed\nand I have already done the reflog commit,   have to manually uncommit\n...\".\nAnd this quickly becomes quite burdensome for consumers.\n\nI think a transaction API should remove this burden from consumers and\nmake it as easy as possible to use the API.\n\nConditional of if it is desireable to have transactions for reflogs at all.\n\n\n\nAbout the cleanup part. The current API, and I think also the current\ndirection of both my old patches (which I think did not go far enough\nin the transactifications) or this current patchseries is that they\nall\nhave a very confusing and inconsistent API for reflog updates.\nWith this I mean,   sometimes reflog updates happen within a\ntransaction as a side effect of a ref_transaction_update().\nOther times reflog updates happens ooutside of transactions by calling\na special reflog API.\n\nI.e.  reflogs sometimes update as part of a transaction and sometimes not.\nA follow up question then on this API is what should happen if you\nhave a transaction open, but not committed, and while the transaction\nis open you call the non-transactional reflog API for a reflog for the\nsame ref that is already beeing/or going to be/ updated as the\nref-update-side-effect ?\n\n\nI think an api where \"sometimes you operate on foo from within a\ntransaction and sometimes you do not, and if you do the latter when a\ntransaction is open, it is unclear what should happen\" is not great.\nIMHO, refs and reflog updates are related and I think:\n\n* a transaction should be the ONLY way to mutate either a ref or a\nreflog or both.\n* if you update both a ref and a reflog then that should happen as\npart of one single transaction.\n* (later) it would probably make the API better if the code was\nrefactored so write_ref_sha1() will NOT call log_ref_write() anymore\nand instead make the reflog update that happens explicit perhaps by\ncalling something like a ref_transaction_update_reflog() as part of\nthe ref_transaction_update() call.\n\n\n\n\n\n\n> Michael\n>\n> [1] The reflog updates are not atomic in the face of disk errors and\n> power outages and stuff, but neither are reference updates. In other\n> words, I think reflog updates in ref_transaction_commit() are done as\n> safely as reference updates, which is surely good enough for this\n> reference backend. Other reference backends can do a better job with\n> both while staying within the existing API.\n>\n> --\n> Michael Haggerty\n> mhagger@alum.mit.edu\n>\n"},{"id":"253687","messageId":"548E1A71.7080707@alum.mit.edu","threadId":"38126","inReplyTo":"CAN05THRiMiQCqO1VZX7zQeWLBZMPG_sx1aVvdGirAoEh2Hd9Dw@mail.gmail.com","subject":"Re: [PATCH 0/8] Making reflog modifications part of the transactions API","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-14T23:17:05Z","receivedAt":"2014-12-14T23:17:05Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 12/12/2014 10:16 PM, ronnie sahlberg wrote:\n> On Fri, Dec 12, 2014 at 11:17 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n>> [...]\n>> What am I missing?\n> \n> My original idea was to clean up a bit of the reflog handling API and\n> have one single transaction API for all  ref and reflog operations.\n> \n> Think future use cases where you have a database backend for both refs\n> and reflogs. It would be very nice to have a single atomic transaction\n> that would either commit or fail atomically any update to refs and/or\n> reflogs.\n> Otherwise you would have all consumers of the API have to invent their\n> own transaction and rewind support : 'oh the ref transaction failed\n> and I have already done the reflog commit,   have to manually uncommit\n> ...\".\n> And this quickly becomes quite burdensome for consumers.\n> \n> I think a transaction API should remove this burden from consumers and\n> make it as easy as possible to use the API.\n> \n> Conditional of if it is desireable to have transactions for reflogs at all.\n\nNobody is against ACID. But the API to trigger an ACID update doesn't\nalways have to look like\n\n    ref_transaction_begin()\n    ref_transaction_update_XXX()\n    /* ... */\n    ref_transaction_commit()\n\nThe reflog_expire() function that I wrote does everything within an\ninternal transaction that the caller doesn't have to know about.\nSimilarly, the reflog update that happens when a reference is updated\nalso occurs within a transaction, even without the caller having to ask\nfor the reflog update explicitly.\n\n> About the cleanup part. The current API, and I think also the current\n> direction of both my old patches (which I think did not go far enough\n> in the transactifications) or this current patchseries is that they\n> all\n> have a very confusing and inconsistent API for reflog updates.\n> With this I mean,   sometimes reflog updates happen within a\n> transaction as a side effect of a ref_transaction_update().\n> Other times reflog updates happens ooutside of transactions by calling\n> a special reflog API.\n> \n> I.e.  reflogs sometimes update as part of a transaction and sometimes not.\n> A follow up question then on this API is what should happen if you\n> have a transaction open, but not committed, and while the transaction\n> is open you call the non-transactional reflog API for a reflog for the\n> same ref that is already beeing/or going to be/ updated as the\n> ref-update-side-effect ?\n\nThe same thing happens as when two independent processes try to update\nthe same reference simultaneously: one fails. But there is currently no\nneed for any command that would do such a thing.\n\n> I think an api where \"sometimes you operate on foo from within a\n> transaction and sometimes you do not, and if you do the latter when a\n> transaction is open, it is unclear what should happen\" is not great.\n> IMHO, refs and reflog updates are related and I think:\n> \n> * a transaction should be the ONLY way to mutate either a ref or a\n> reflog or both.\n> * if you update both a ref and a reflog then that should happen as\n> part of one single transaction.\n> * (later) it would probably make the API better if the code was\n> refactored so write_ref_sha1() will NOT call log_ref_write() anymore\n> and instead make the reflog update that happens explicit perhaps by\n> calling something like a ref_transaction_update_reflog() as part of\n> the ref_transaction_update() call.\n\nI disagree. The reflog should be updated *whenever* a reference is\nupdated (for references that have reflogs enabled). So why should\ncallers have to remember to trigger the reflog update as an extra step?\nIt's better that the reflog update is an intrinsic part of updating the\nreference; that way nobody can forget it.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"}]}