{"thread":{"id":"37266","subject":"[PATCH 1/5] refs.c: replace the onerr argument in update_ref with a strbuf err","startedAt":"2014-07-31T21:25:05Z","lastAt":"2014-08-08T16:15:29Z","messageCount":7,"participants":["Ronnie Sahlberg"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"247078","messageId":"1406841910-3278-1-git-send-email-sahlberg@google.com","threadId":"37266","inReplyTo":null,"subject":"[PATCH 0/5] ref-transactions-req-strbuf-err","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-31T21:25:05Z","receivedAt":"2014-07-31T21:25:05Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"List,\n\nThis is the next patch series in the ref transaction work.\nThis patch series is called ref-transactions-req-strbuf-err and builds ontop\nof the series called ref-transactions-req-packed-refs which is origin/pu\n\n\nThis patch series mainly adds some nice strbuf arguments to some functions to\npass errors back to callers.\nThe only thing noteworthy is that we finally get to remove\n-enum action_on_err {\n-       UPDATE_REFS_MSG_ON_ERR,\n-       UPDATE_REFS_DIE_ON_ERR,\n-       UPDATE_REFS_QUIET_ON_ERR\n-};\n\naside from that there is little/nothing much interesting in there.\n\n\nRonnie Sahlberg (5):\n  refs.c: replace the onerr argument in update_ref with a strbuf err\n  refs.c: make add_packed_ref return an error instead of calling die\n  refs.c: make lock_packed_refs take an err argument\n  refs.c: add an err argument to commit_packed_refs\n  refs.c: add an err argument to pack_refs\n\n builtin/checkout.c   |   7 ++-\n builtin/clone.c      |  23 +++++---\n builtin/merge.c      |  20 ++++---\n builtin/notes.c      |  24 +++++----\n builtin/pack-refs.c  |   8 ++-\n builtin/reset.c      |  12 +++--\n builtin/update-ref.c |   7 ++-\n notes-cache.c        |   2 +-\n notes-utils.c        |   5 +-\n refs.c               | 148 +++++++++++++++++++++++++++++----------------------\n refs.h               |  13 ++---\n transport-helper.c   |   7 ++-\n transport.c          |   9 ++--\n 13 files changed, 170 insertions(+), 115 deletions(-)\n\n-- \n2.0.1.523.g70700c9\n"},{"id":"247077","messageId":"1406841910-3278-2-git-send-email-sahlberg@google.com","threadId":"37266","inReplyTo":"1406841910-3278-1-git-send-email-sahlberg@google.com","subject":"[PATCH 1/5] refs.c: replace the onerr argument in update_ref with a strbuf err","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-31T21:25:06Z","receivedAt":"2014-07-31T21:25:06Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/checkout.c   |  7 +++++--\n builtin/clone.c      | 23 +++++++++++++++--------\n builtin/merge.c      | 20 +++++++++++++-------\n builtin/notes.c      | 24 ++++++++++++++----------\n builtin/reset.c      | 12 ++++++++----\n builtin/update-ref.c |  7 +++++--\n notes-cache.c        |  2 +-\n notes-utils.c        |  5 +++--\n refs.c               | 14 +++-----------\n refs.h               | 10 ++--------\n transport-helper.c   |  7 ++++++-\n transport.c          |  9 ++++++---\n 12 files changed, 81 insertions(+), 59 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 808c58f..a9ec5be 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -580,6 +580,8 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n {\n \tstruct strbuf msg = STRBUF_INIT;\n \tconst char *old_desc, *reflog_msg;\n+\tstruct strbuf err = STRBUF_INIT;\n+\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@@ -617,8 +619,9 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n \tif (!strcmp(new->name, \"HEAD\") && !new->path && !opts->force_detach) {\n \t\t/* Nothing to do. */\n \t} else if (opts->force_detach || !new->path) {\t/* No longer on any branch. */\n-\t\tupdate_ref(msg.buf, \"HEAD\", new->commit->object.sha1, NULL,\n-\t\t\t   REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n+\t\tif (update_ref(msg.buf, \"HEAD\", new->commit->object.sha1, NULL,\n+\t\t\t       REF_NODEREF, &err))\n+\t\t\tdie(\"%s\", err.buf);\n \t\tif (!opts->quiet) {\n \t\t\tif (old->path && advice_detached_head)\n \t\t\t\tdetach_advice(new->name);\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 7737e12..99e23cf 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -521,6 +521,7 @@ static void write_remote_refs(const struct ref *local_refs)\n static void write_followtags(const struct ref *refs, const char *msg)\n {\n \tconst struct ref *ref;\n+\tstruct strbuf err = STRBUF_INIT;\n \tfor (ref = refs; ref; ref = ref->next) {\n \t\tif (!starts_with(ref->name, \"refs/tags/\"))\n \t\t\tcontinue;\n@@ -528,8 +529,9 @@ static void write_followtags(const struct ref *refs, const char *msg)\n \t\t\tcontinue;\n \t\tif (!has_sha1_file(ref->old_sha1))\n \t\t\tcontinue;\n-\t\tupdate_ref(msg, ref->name, ref->old_sha1,\n-\t\t\t   NULL, 0, UPDATE_REFS_DIE_ON_ERR);\n+\t\tif (update_ref(msg, ref->name, ref->old_sha1,\n+\t\t\t       NULL, 0, &err))\n+\t\t\tdie(\"%s\", err.buf);\n \t}\n }\n \n@@ -592,28 +594,33 @@ static void update_remote_refs(const struct ref *refs,\n static void update_head(const struct ref *our, const struct ref *remote,\n \t\t\tconst char *msg)\n {\n+\tstruct strbuf err = STRBUF_INIT;\n+\n \tif (our && starts_with(our->name, \"refs/heads/\")) {\n \t\t/* Local default branch link */\n \t\tcreate_symref(\"HEAD\", our->name, NULL);\n \t\tif (!option_bare) {\n \t\t\tconst char *head = skip_prefix(our->name, \"refs/heads/\");\n-\t\t\tupdate_ref(msg, \"HEAD\", our->old_sha1, NULL, 0,\n-\t\t\t\t   UPDATE_REFS_DIE_ON_ERR);\n+\t\t\tif (update_ref(msg, \"HEAD\", our->old_sha1, NULL, 0,\n+\t\t\t\t       &err))\n+\t\t\t\tdie(\"%s\", err.buf);\n \t\t\tinstall_branch_config(0, head, option_origin, our->name);\n \t\t}\n \t} else if (our) {\n \t\tstruct commit *c = lookup_commit_reference(our->old_sha1);\n \t\t/* --branch specifies a non-branch (i.e. tags), detach HEAD */\n-\t\tupdate_ref(msg, \"HEAD\", c->object.sha1,\n-\t\t\t   NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n+\t\tif (update_ref(msg, \"HEAD\", c->object.sha1,\n+\t\t\t       NULL, REF_NODEREF, &err))\n+\t\t\tdie(\"%s\", err.buf);\n \t} else if (remote) {\n \t\t/*\n \t\t * We know remote HEAD points to a non-branch, or\n \t\t * HEAD points to a branch but we don't know which one.\n \t\t * Detach HEAD in all these cases.\n \t\t */\n-\t\tupdate_ref(msg, \"HEAD\", remote->old_sha1,\n-\t\t\t   NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n+\t  if (update_ref(msg, \"HEAD\", remote->old_sha1,\n+\t\t\t NULL, REF_NODEREF, &err))\n+\t\tdie(\"%s\", err.buf);\n \t}\n }\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 428ca24..17dda64 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -396,9 +396,11 @@ static void finish(struct commit *head_commit,\n \t\t\tprintf(_(\"No merge message -- not updating HEAD\\n\"));\n \t\telse {\n \t\t\tconst char *argv_gc_auto[] = { \"gc\", \"--auto\", NULL };\n-\t\t\tupdate_ref(reflog_message.buf, \"HEAD\",\n-\t\t\t\tnew_head, head, 0,\n-\t\t\t\tUPDATE_REFS_DIE_ON_ERR);\n+\t\t\tstruct strbuf err = STRBUF_INIT;\n+\t\t\tif (update_ref(reflog_message.buf, \"HEAD\",\n+\t\t\t\t       new_head, head, 0,\n+\t\t\t\t       &err))\n+\t\t\t\tdie(\"%s\", err.buf);\n \t\t\t/*\n \t\t\t * We ignore errors in 'gc --auto', since the\n \t\t\t * user should see them.\n@@ -1093,6 +1095,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tunsigned char head_sha1[20];\n \tstruct commit *head_commit;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \tconst char *head_arg;\n \tint flag, i, ret = 0, head_subsumed;\n \tint best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;\n@@ -1221,8 +1224,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tif (!remote_head)\n \t\t\tdie(_(\"%s - not something we can merge\"), argv[0]);\n \t\tread_empty(remote_head->object.sha1, 0);\n-\t\tupdate_ref(\"initial pull\", \"HEAD\", remote_head->object.sha1,\n-\t\t\t   NULL, 0, UPDATE_REFS_DIE_ON_ERR);\n+\t\tif (update_ref(\"initial pull\", \"HEAD\", remote_head->object.sha1,\n+\t\t\t       NULL, 0, &err))\n+\t\t\tdie(\"%s\", err.buf);\n \t\tgoto done;\n \t} else {\n \t\tstruct strbuf merge_names = STRBUF_INIT;\n@@ -1338,8 +1342,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tfree(list);\n \t}\n \n-\tupdate_ref(\"updating ORIG_HEAD\", \"ORIG_HEAD\", head_commit->object.sha1,\n-\t\t   NULL, 0, UPDATE_REFS_DIE_ON_ERR);\n+\tif (update_ref(\"updating ORIG_HEAD\", \"ORIG_HEAD\",\n+\t\t       head_commit->object.sha1,\n+\t\t       NULL, 0, &err))\n+\t\tdie(\"%s\", err.buf);\n \n \tif (remoteheads && !common)\n \t\t; /* No common ancestors found. We need a real merge. */\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 820c341..f508fbd 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -675,6 +675,7 @@ static int merge_abort(struct notes_merge_options *o)\n static int merge_commit(struct notes_merge_options *o)\n {\n \tstruct strbuf msg = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \tunsigned char sha1[20], parent_sha1[20];\n \tstruct notes_tree *t;\n \tstruct commit *partial;\n@@ -715,10 +716,10 @@ static int merge_commit(struct notes_merge_options *o)\n \tformat_commit_message(partial, \"%s\", &msg, &pretty_ctx);\n \tstrbuf_trim(&msg);\n \tstrbuf_insert(&msg, 0, \"notes: \", 7);\n-\tupdate_ref(msg.buf, o->local_ref, sha1,\n-\t\t   is_null_sha1(parent_sha1) ? NULL : parent_sha1,\n-\t\t   0, UPDATE_REFS_DIE_ON_ERR);\n-\n+\tif (update_ref(msg.buf, o->local_ref, sha1,\n+\t\t       is_null_sha1(parent_sha1) ? NULL : parent_sha1,\n+\t\t       0, &err))\n+\t\tdie(\"%s\", err.buf);\n \tfree_notes(t);\n \tstrbuf_release(&msg);\n \tret = merge_abort(o);\n@@ -729,6 +730,7 @@ static int merge_commit(struct notes_merge_options *o)\n static int merge(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \tunsigned char result_sha1[20];\n \tstruct notes_tree *t;\n \tstruct notes_merge_options o;\n@@ -809,14 +811,16 @@ static int merge(int argc, const char **argv, const char *prefix)\n \n \tresult = notes_merge(&o, t, result_sha1);\n \n-\tif (result >= 0) /* Merge resulted (trivially) in result_sha1 */\n+\tif (result >= 0) {/* Merge resulted (trivially) in result_sha1 */\n \t\t/* Update default notes ref with new commit */\n-\t\tupdate_ref(msg.buf, default_notes_ref(), result_sha1, NULL,\n-\t\t\t   0, UPDATE_REFS_DIE_ON_ERR);\n-\telse { /* Merge has unresolved conflicts */\n+\t\tif (update_ref(msg.buf, default_notes_ref(), result_sha1, NULL,\n+\t\t\t       0, &err))\n+\t\t\tdie(\"%s\", err.buf);\n+\t} else { /* Merge has unresolved conflicts */\n \t\t/* Update .git/NOTES_MERGE_PARTIAL with partial merge result */\n-\t\tupdate_ref(msg.buf, \"NOTES_MERGE_PARTIAL\", result_sha1, NULL,\n-\t\t\t   0, UPDATE_REFS_DIE_ON_ERR);\n+\t\tif (update_ref(msg.buf, \"NOTES_MERGE_PARTIAL\", result_sha1, NULL,\n+\t\t\t       0, &err))\n+\t\t\tdie(\"%s\", err.buf);\n \t\t/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */\n \t\tif (create_symref(\"NOTES_MERGE_REF\", default_notes_ref(), NULL))\n \t\t\tdie(\"Failed to store link to current notes ref (%s)\",\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex f368266..0e8775d 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -244,6 +244,7 @@ static int reset_refs(const char *rev, const unsigned char *sha1)\n {\n \tint update_ref_status;\n \tstruct strbuf msg = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \tunsigned char *orig = NULL, sha1_orig[20],\n \t\t*old_orig = NULL, sha1_old_orig[20];\n \n@@ -252,13 +253,16 @@ static int reset_refs(const char *rev, const unsigned char *sha1)\n \tif (!get_sha1(\"HEAD\", sha1_orig)) {\n \t\torig = sha1_orig;\n \t\tset_reflog_message(&msg, \"updating ORIG_HEAD\", NULL);\n-\t\tupdate_ref(msg.buf, \"ORIG_HEAD\", orig, old_orig, 0,\n-\t\t\t   UPDATE_REFS_MSG_ON_ERR);\n+\t\tif (update_ref(msg.buf, \"ORIG_HEAD\", orig, old_orig, 0, &err))\n+\t\t\terror(\"%s\", err.buf);\n+\t\tstrbuf_release(&err);\n \t} else if (old_orig)\n \t\tdelete_ref(\"ORIG_HEAD\", old_orig, 0);\n \tset_reflog_message(&msg, \"updating HEAD\", rev);\n-\tupdate_ref_status = update_ref(msg.buf, \"HEAD\", sha1, orig, 0,\n-\t\t\t\t       UPDATE_REFS_MSG_ON_ERR);\n+\tupdate_ref_status = update_ref(msg.buf, \"HEAD\", sha1, orig, 0, &err);\n+\tif (update_ref_status)\n+\t\terror(\"%s\", err.buf);\n+\tstrbuf_release(&err);\n \tstrbuf_release(&msg);\n \treturn update_ref_status;\n }\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex d62871d..062a29c 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -349,6 +349,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \tconst char *refname, *oldval;\n \tunsigned char sha1[20], oldsha1[20];\n \tint delete = 0, no_deref = 0, read_stdin = 0, end_null = 0, flags = 0;\n+\tstruct strbuf err = STRBUF_INIT;\n \tstruct option options[] = {\n \t\tOPT_STRING( 'm', NULL, &msg, N_(\"reason\"), N_(\"reason of the update\")),\n \t\tOPT_BOOL('d', NULL, &delete, N_(\"delete the reference\")),\n@@ -406,6 +407,8 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \tif (delete)\n \t\treturn delete_ref(refname, oldval ? oldsha1 : NULL, flags);\n \telse\n-\t\treturn update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,\n-\t\t\t\t  flags, UPDATE_REFS_DIE_ON_ERR);\n+\t\tif (update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,\n+\t\t\t       flags, &err))\n+\t\t\tdie(\"%s\", err.buf);\n+\treturn 0;\n }\ndiff --git a/notes-cache.c b/notes-cache.c\nindex 97dfd63..882b56d 100644\n--- a/notes-cache.c\n+++ b/notes-cache.c\n@@ -62,7 +62,7 @@ int notes_cache_write(struct notes_cache *c)\n \tif (commit_tree(&msg, tree_sha1, NULL, commit_sha1, NULL, NULL) < 0)\n \t\treturn -1;\n \tif (update_ref(\"update notes cache\", c->tree.ref, commit_sha1, NULL,\n-\t\t       0, UPDATE_REFS_QUIET_ON_ERR) < 0)\n+\t\t       0, NULL) < 0)\n \t\treturn -1;\n \n \treturn 0;\ndiff --git a/notes-utils.c b/notes-utils.c\nindex a0b1d7b..805bc31 100644\n--- a/notes-utils.c\n+++ b/notes-utils.c\n@@ -33,6 +33,7 @@ void commit_notes(struct notes_tree *t, const char *msg)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tunsigned char commit_sha1[20];\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tif (!t)\n \t\tt = &default_notes_tree;\n@@ -48,8 +49,8 @@ void commit_notes(struct notes_tree *t, const char *msg)\n \n \tcreate_notes_commit(t, NULL, &buf, commit_sha1);\n \tstrbuf_insert(&buf, 0, \"notes: \", 7); /* commit message starts at index 7 */\n-\tupdate_ref(buf.buf, t->ref, commit_sha1, NULL, 0,\n-\t\t   UPDATE_REFS_DIE_ON_ERR);\n+\tif (update_ref(buf.buf, t->ref, commit_sha1, NULL, 0, &err))\n+\t\tdie(\"%s\", err.buf);\n \n \tstrbuf_release(&buf);\n }\ndiff --git a/refs.c b/refs.c\nindex 46a9595..65eee72 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3549,7 +3549,7 @@ int transaction_delete_sha1(struct ref_transaction *transaction,\n \n 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+\t       int flags, struct strbuf *e)\n {\n \tstruct ref_transaction *t;\n \tstruct strbuf err = STRBUF_INIT;\n@@ -3562,16 +3562,8 @@ int update_ref(const char *action, const char *refname,\n \t\tconst char *str = \"update_ref failed for ref '%s': %s\";\n \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-\t\t\tbreak;\n-\t\tcase UPDATE_REFS_DIE_ON_ERR:\n-\t\t\tdie(str, refname, err.buf);\n-\t\t\tbreak;\n-\t\tcase UPDATE_REFS_QUIET_ON_ERR:\n-\t\t\tbreak;\n-\t\t}\n+\t\tif (e)\n+\t\t\tstrbuf_addf(e, str, refname, err.buf);\n \t\tstrbuf_release(&err);\n \t\treturn 1;\n \t}\ndiff --git a/refs.h b/refs.h\nindex 54dbe4b..dee9a98 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -202,12 +202,6 @@ extern int rename_ref(const char *oldref, const char *newref, const char *logmsg\n  */\n extern int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sha1);\n \n-enum action_on_err {\n-\tUPDATE_REFS_MSG_ON_ERR,\n-\tUPDATE_REFS_DIE_ON_ERR,\n-\tUPDATE_REFS_QUIET_ON_ERR\n-};\n-\n /*\n  * Begin a reference transaction.  The reference transaction must\n  * be freed by calling transaction_free().\n@@ -332,8 +326,8 @@ void transaction_free(struct ref_transaction *transaction);\n \n /** Lock a ref and then write its file */\n int update_ref(const char *action, const char *refname,\n-\t\tconst unsigned char *sha1, const unsigned char *oldval,\n-\t\tint flags, enum action_on_err onerr);\n+\t       const unsigned char *sha1, const unsigned char *oldval,\n+\t       int flags, struct strbuf *err);\n \n extern int parse_hide_refs_config(const char *var, const char *value, const char *);\n extern int ref_is_hidden(const char *);\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 270ae28..d46624a 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -733,6 +733,7 @@ static int push_update_refs_status(struct helper_data *data,\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct ref *ref = remote_refs;\n+\tstruct strbuf err = STRBUF_INIT;\n \tint ret = 0;\n \n \tfor (;;) {\n@@ -756,7 +757,11 @@ static int push_update_refs_status(struct helper_data *data,\n \t\tprivate = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);\n \t\tif (!private)\n \t\t\tcontinue;\n-\t\tupdate_ref(\"update by helper\", private, ref->new_sha1, NULL, 0, 0);\n+\t\tif (update_ref(\"update by helper\", private, ref->new_sha1,\n+\t\t\t       NULL, 0, &err))\n+\t\t\terror(\"%s\", err.buf);\n+\t\tstrbuf_release(&err);\n+\n \t\tfree(private);\n \t}\n \tstrbuf_release(&buf);\ndiff --git a/transport.c b/transport.c\nindex f40e950..a3b7f48 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -607,6 +607,7 @@ int transport_refs_pushed(struct ref *ref)\n void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int verbose)\n {\n \tstruct refspec rs;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tif (ref->status != REF_STATUS_OK && ref->status != REF_STATUS_UPTODATE)\n \t\treturn;\n@@ -619,9 +620,11 @@ void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int v\n \t\t\tfprintf(stderr, \"updating local tracking ref '%s'\\n\", rs.dst);\n \t\tif (ref->deletion) {\n \t\t\tdelete_ref(rs.dst, NULL, 0);\n-\t\t} else\n-\t\t\tupdate_ref(\"update by push\", rs.dst,\n-\t\t\t\t\tref->new_sha1, NULL, 0, 0);\n+\t\t} else if (update_ref(\"update by push\", rs.dst,\n+\t\t\t\t      ref->new_sha1, NULL, 0, &err))\n+\t\t\terror(\"%s\", err.buf);\n+\n+\t\tstrbuf_release(&err);\n \t\tfree(rs.dst);\n \t}\n }\n-- \n2.0.1.523.g70700c9\n"},{"id":"247081","messageId":"1406841910-3278-3-git-send-email-sahlberg@google.com","threadId":"37266","inReplyTo":"1406841910-3278-1-git-send-email-sahlberg@google.com","subject":"[PATCH 2/5] refs.c: make add_packed_ref return an error instead of calling die","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-31T21:25:07Z","receivedAt":"2014-07-31T21:25:07Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change add_packed_ref to return an error instead of calling die().\nUpdate all callers to check the return value of add_packed_ref.\n\nWe can also skip checking the refname format since this function is now\nstatic and only called from the transaction code.\nIf we are updating a ref and the refname is bad then we fail the transaction\nalready in transaction_update_sha1().\nFor the ref deletion case the only caveat is that we would not want\nto move the badly named ref into the packed refs file during transaction\ncommit. This again is not a problem since if the name is bad, then\nresolve_ref_unsafe() will fail which protects us from calling add_packed_ref()\nwith the bad name.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 23 +++++++++++++++++------\n 1 file changed, 17 insertions(+), 6 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 65eee72..0aad8c8 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1135,17 +1135,16 @@ static struct ref_dir *get_packed_refs(struct ref_cache *refs)\n \treturn get_packed_ref_dir(get_packed_ref_cache(refs));\n }\n \n-static void add_packed_ref(const char *refname, const unsigned char *sha1)\n+static int add_packed_ref(const char *refname, const unsigned char *sha1)\n {\n \tstruct packed_ref_cache *packed_ref_cache =\n \t\tget_packed_ref_cache(&ref_cache);\n \n \tif (!packed_ref_cache->lock)\n-\t\tdie(\"internal error: packed refs not locked\");\n-\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))\n-\t\tdie(\"Reference has invalid format: '%s'\", refname);\n+\t\treturn -1;\n \tadd_ref(get_packed_ref_dir(packed_ref_cache),\n \t\tcreate_ref_entry(refname, sha1, REF_ISPACKED));\n+\treturn 0;\n }\n \n /*\n@@ -3666,7 +3665,13 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t\t\t\tRESOLVE_REF_READING, NULL))\n \t\t\tcontinue;\n \n-\t\tadd_packed_ref(update->refname, sha1);\n+\t\tif (add_packed_ref(update->refname, sha1)) {\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(err, \"Failed to add %s to packed \"\n+\t\t\t\t\t    \"refs\", update->refname);\n+\t\t\tret = -1;\n+\t\t\tgoto cleanup;\n+\t\t}\n \t\tneed_repack = 1;\n \t}\n \tif (need_repack) {\n@@ -3778,7 +3783,13 @@ int transaction_commit(struct ref_transaction *transaction,\n \n \t\tpacked = get_packed_refs(&ref_cache);\n \t\tremove_entry(packed, update->refname);\n-\t\tadd_packed_ref(update->refname, update->new_sha1);\n+\t\tif (add_packed_ref(update->refname, update->new_sha1)) {\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(err, \"Failed to add %s to packed \"\n+\t\t\t\t\t    \"refs\", update->refname);\n+\t\t\tret = -1;\n+\t\t\tgoto cleanup;\n+\t\t}\n \t\tneed_repack = 1;\n \n \t\ttry_remove_empty_parents((char *)update->refname);\n-- \n2.0.1.523.g70700c9\n"},{"id":"247080","messageId":"1406841910-3278-4-git-send-email-sahlberg@google.com","threadId":"37266","inReplyTo":"1406841910-3278-1-git-send-email-sahlberg@google.com","subject":"[PATCH 3/5] refs.c: make lock_packed_refs take an err argument","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-31T21:25:08Z","receivedAt":"2014-07-31T21:25:08Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 25 +++++++++++++------------\n 1 file changed, 13 insertions(+), 12 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 0aad8c8..cfe1292 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2270,13 +2270,17 @@ static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)\n \treturn 0;\n }\n \n-/* This should return a meaningful errno on failure */\n-static int lock_packed_refs(int flags)\n+static int lock_packed_refs(struct strbuf *err)\n {\n \tstruct packed_ref_cache *packed_ref_cache;\n \n-\tif (hold_lock_file_for_update(&packlock, git_path(\"packed-refs\"), flags) < 0)\n+\tif (hold_lock_file_for_update(&packlock, git_path(\"packed-refs\"),\n+\t\t\t\t      0) < 0) {\n+\t\tif (err)\n+\t\t\tunable_to_lock_message(git_path(\"packed-refs\"),\n+\t\t\t\t\t       errno, err);\n \t\treturn -1;\n+\t}\n \t/*\n \t * Get the current packed-refs while holding the lock.  If the\n \t * packed-refs file has been modified since we last read it,\n@@ -2460,11 +2464,14 @@ static void prune_refs(struct ref_to_prune *r)\n int pack_refs(unsigned int flags)\n {\n \tstruct pack_refs_cb_data cbdata;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tmemset(&cbdata, 0, sizeof(cbdata));\n \tcbdata.flags = flags;\n \n-\tlock_packed_refs(LOCK_DIE_ON_ERROR);\n+\tif (lock_packed_refs(&err))\n+\t\tdie(\"%s\", err.buf);\n+\n \tcbdata.packed_refs = get_packed_refs(&ref_cache);\n \n \tdo_for_each_entry_in_dir(get_loose_refs(&ref_cache), 0,\n@@ -3626,10 +3633,7 @@ int transaction_commit(struct ref_transaction *transaction,\n \t}\n \n \t/* Lock packed refs during commit */\n-\tif (lock_packed_refs(0)) {\n-\t\tif (err)\n-\t\t\tunable_to_lock_message(git_path(\"packed-refs\"),\n-\t\t\t\t\t       errno, err);\n+\tif (lock_packed_refs(err)) {\n \t\tret = -1;\n \t\tgoto cleanup;\n \t}\n@@ -3684,10 +3688,7 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t\tgoto cleanup;\n \t\t}\n \t\t/* lock the packed refs again so no one can change it */\n-\t\tif (lock_packed_refs(0)) {\n-\t\t\tif (err)\n-\t\t\t\tunable_to_lock_message(git_path(\"packed-refs\"),\n-\t\t\t\t\t\t       errno, err);\n+\t\tif (lock_packed_refs(err)) {\n \t\t\tret = -1;\n \t\t\tgoto cleanup;\n \t\t}\n-- \n2.0.1.523.g70700c9\n"},{"id":"247079","messageId":"1406841910-3278-5-git-send-email-sahlberg@google.com","threadId":"37266","inReplyTo":"1406841910-3278-1-git-send-email-sahlberg@google.com","subject":"[PATCH 4/5] refs.c: add an err argument to commit_packed_refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-31T21:25:09Z","receivedAt":"2014-07-31T21:25:09Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 85 +++++++++++++++++++++++++++++++++++++++---------------------------\n 1 file changed, 50 insertions(+), 35 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex cfe1292..19e73f3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2232,8 +2232,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\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.\n  */\n-static void write_packed_entry(int fd, char *refname, unsigned char *sha1,\n-\t\t\t       unsigned char *peeled)\n+static int write_packed_entry(int fd, char *refname, unsigned char *sha1,\n+\t\t\t      unsigned char *peeled, struct strbuf *err)\n {\n \tchar line[PATH_MAX + 100];\n \tint len;\n@@ -2241,33 +2241,50 @@ static void write_packed_entry(int fd, char *refname, unsigned char *sha1,\n \tlen = snprintf(line, sizeof(line), \"%s %s\\n\",\n \t\t       sha1_to_hex(sha1), refname);\n \t/* this should not happen but just being defensive */\n-\tif (len > sizeof(line))\n-\t\tdie(\"too long a refname '%s'\", refname);\n-\twrite_or_die(fd, line, len);\n-\n+\tif (len > sizeof(line)) {\n+\t\tstrbuf_addf(err, \"too long a refname '%s'\", refname);\n+\t\treturn -1;\n+\t}\n+\tif (write_in_full(fd, line, len) != len) {\n+\t\tstrbuf_addf(err, \"error writing to packed-refs. %s\",\n+\t\t\t    strerror(errno));\n+\t\treturn -1;\n+\t}\n \tif (peeled) {\n \t\tif (snprintf(line, sizeof(line), \"^%s\\n\",\n \t\t\t     sha1_to_hex(peeled)) != PEELED_LINE_LENGTH)\n \t\t\tdie(\"internal error\");\n-\t\twrite_or_die(fd, line, PEELED_LINE_LENGTH);\n+\t\tlen = PEELED_LINE_LENGTH;\n+\t\tif (write_in_full(fd, line, len) != len) {\n+\t\t\tstrbuf_addf(err, \"error writing to packed-refs. %s\",\n+\t\t\t\t    strerror(errno));\n+\t\t\treturn -1;\n+\t\t}\n \t}\n+\treturn 0;\n }\n \n+struct write_packed_data {\n+\tint fd;\n+\tstruct strbuf *err;\n+};\n+\n /*\n  * An each_ref_entry_fn that writes the entry to a packed-refs file.\n  */\n static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)\n {\n-\tint *fd = cb_data;\n+\tstruct write_packed_data *data = cb_data;\n \tenum peel_status peel_status = peel_entry(entry, 0);\n \n-\tif (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)\n-\t\terror(\"internal error: %s is not a valid packed reference!\",\n-\t\t      entry->name);\n-\twrite_packed_entry(*fd, entry->name, entry->u.value.sha1,\n-\t\t\t   peel_status == PEEL_PEELED ?\n-\t\t\t   entry->u.value.peeled : NULL);\n-\treturn 0;\n+\tif (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG) {\n+\t\tstrbuf_addf(data->err, \"internal error: %s is not a \"\n+\t\t\t    \"valid packed reference!\", entry->name);\n+\t\treturn -1;\n+\t}\n+\treturn write_packed_entry(data->fd, entry->name, entry->u.value.sha1,\n+\t\t\t\t  peel_status == PEEL_PEELED ?\n+\t\t\t\t  entry->u.value.peeled : NULL, data->err);\n }\n \n static int lock_packed_refs(struct strbuf *err)\n@@ -2296,30 +2313,34 @@ static int lock_packed_refs(struct strbuf *err)\n \n /*\n  * Commit the packed refs changes.\n- * On error we must make sure that errno contains a meaningful value.\n  */\n-static int commit_packed_refs(void)\n+static int commit_packed_refs(struct strbuf *err)\n {\n \tstruct packed_ref_cache *packed_ref_cache =\n \t\tget_packed_ref_cache(&ref_cache);\n \tint error = 0;\n-\tint save_errno = 0;\n+\tstruct write_packed_data data;\n \n \tif (!packed_ref_cache->lock)\n \t\tdie(\"internal error: packed-refs not locked\");\n-\twrite_or_die(packed_ref_cache->lock->fd,\n-\t\t     PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));\n+\tif (write_in_full(packed_ref_cache->lock->fd,\n+\t\t\t  PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER)) < 0) {\n+\t\tstrbuf_addf(err, \"error writing packed-refs. %s\",\n+\t\t\t    strerror(errno));\n+\t\treturn -1;\n+       }\n \n+\tdata.fd = packed_ref_cache->lock->fd;\n+\tdata.err = err;\n \tdo_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),\n-\t\t\t\t 0, write_packed_entry_fn,\n-\t\t\t\t &packed_ref_cache->lock->fd);\n+\t\t\t\t 0, write_packed_entry_fn, &data);\n \tif (commit_lock_file(packed_ref_cache->lock)) {\n-\t\tsave_errno = errno;\n+\t\tstrbuf_addf(err, \"unable to overwrite old ref-pack \"\n+\t\t\t    \"file. %s\", strerror(errno));\n \t\terror = -1;\n \t}\n \tpacked_ref_cache->lock = NULL;\n \trelease_packed_ref_cache(packed_ref_cache);\n-\terrno = save_errno;\n \treturn error;\n }\n \n@@ -2477,8 +2498,8 @@ int pack_refs(unsigned int flags)\n \tdo_for_each_entry_in_dir(get_loose_refs(&ref_cache), 0,\n \t\t\t\t pack_if_possible_fn, &cbdata);\n \n-\tif (commit_packed_refs())\n-\t\tdie_errno(\"unable to overwrite old ref-pack file\");\n+\tif (commit_packed_refs(&err))\n+\t\tdie(\"%s\", err.buf);\n \n \tprune_refs(cbdata.ref_to_prune);\n \treturn 0;\n@@ -2549,7 +2570,7 @@ static int repack_without_refs(const char **refnames, int n, struct strbuf *err)\n \tstruct ref_dir *packed;\n \tstruct string_list refs_to_delete = STRING_LIST_INIT_DUP;\n \tstruct string_list_item *ref_to_delete;\n-\tint i, ret;\n+\tint i;\n \n \t/* Look for a packed ref */\n \tfor (i = 0; i < n; i++)\n@@ -2572,11 +2593,7 @@ static int repack_without_refs(const char **refnames, int n, struct strbuf *err)\n \t}\n \n \t/* Write what remains */\n-\tret = commit_packed_refs();\n-\tif (ret && err)\n-\t\tstrbuf_addf(err, \"unable to overwrite old ref-pack file: %s\",\n-\t\t\t    strerror(errno));\n-\treturn ret;\n+\treturn commit_packed_refs(err);\n }\n \n static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\n@@ -3681,9 +3698,7 @@ int transaction_commit(struct ref_transaction *transaction,\n \tif (need_repack) {\n \t\tpacked = get_packed_refs(&ref_cache);\n \t\tsort_ref_dir(packed);\n-\t\tif (commit_packed_refs()){\n-\t\t\tstrbuf_addf(err, \"unable to overwrite old ref-pack \"\n-\t\t\t\t    \"file\");\n+\t\tif (commit_packed_refs(err)){\n \t\t\tret = -1;\n \t\t\tgoto cleanup;\n \t\t}\n-- \n2.0.1.523.g70700c9\n"},{"id":"247082","messageId":"1406841910-3278-6-git-send-email-sahlberg@google.com","threadId":"37266","inReplyTo":"1406841910-3278-1-git-send-email-sahlberg@google.com","subject":"[PATCH 5/5] refs.c: add an err argument to pack_refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-31T21:25:10Z","receivedAt":"2014-07-31T21:25:10Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/pack-refs.c |  8 +++++++-\n refs.c              | 13 ++++++-------\n refs.h              |  3 ++-\n 3 files changed, 15 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/pack-refs.c b/builtin/pack-refs.c\nindex b20b1ec..da5d46a 100644\n--- a/builtin/pack-refs.c\n+++ b/builtin/pack-refs.c\n@@ -10,6 +10,7 @@ static char const * const pack_refs_usage[] = {\n int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n {\n \tunsigned int flags = PACK_REFS_PRUNE;\n+\tstruct strbuf err = STRBUF_INIT;\n \tstruct option opts[] = {\n \t\tOPT_BIT(0, \"all\",   &flags, N_(\"pack everything\"), PACK_REFS_ALL),\n \t\tOPT_BIT(0, \"prune\", &flags, N_(\"prune loose refs (default)\"), PACK_REFS_PRUNE),\n@@ -17,5 +18,10 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n \t};\n \tif (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))\n \t\tusage_with_options(pack_refs_usage, opts);\n-\treturn pack_refs(flags);\n+\tif (pack_refs(flags, &err)) {\n+\t\terror(\"%s\", err.buf);\n+\t\tstrbuf_release(&err);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n }\ndiff --git a/refs.c b/refs.c\nindex 19e73f3..5875c29 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2328,7 +2328,7 @@ static int commit_packed_refs(struct strbuf *err)\n \t\tstrbuf_addf(err, \"error writing packed-refs. %s\",\n \t\t\t    strerror(errno));\n \t\treturn -1;\n-       }\n+\t}\n \n \tdata.fd = packed_ref_cache->lock->fd;\n \tdata.err = err;\n@@ -2482,24 +2482,23 @@ static void prune_refs(struct ref_to_prune *r)\n \t}\n }\n \n-int pack_refs(unsigned int flags)\n+int pack_refs(unsigned int flags, struct strbuf *err)\n {\n \tstruct pack_refs_cb_data cbdata;\n-\tstruct strbuf err = STRBUF_INIT;\n \n \tmemset(&cbdata, 0, sizeof(cbdata));\n \tcbdata.flags = flags;\n \n-\tif (lock_packed_refs(&err))\n-\t\tdie(\"%s\", err.buf);\n+\tif (lock_packed_refs(err))\n+\t\treturn -1;\n \n \tcbdata.packed_refs = get_packed_refs(&ref_cache);\n \n \tdo_for_each_entry_in_dir(get_loose_refs(&ref_cache), 0,\n \t\t\t\t pack_if_possible_fn, &cbdata);\n \n-\tif (commit_packed_refs(&err))\n-\t\tdie(\"%s\", err.buf);\n+\tif (commit_packed_refs(err))\n+\t\treturn -1;\n \n \tprune_refs(cbdata.ref_to_prune);\n \treturn 0;\ndiff --git a/refs.h b/refs.h\nindex dee9a98..1a98e27 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -122,8 +122,9 @@ extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct st\n /*\n  * Write a packed-refs file for the current repository.\n  * flags: Combination of the above PACK_REFS_* flags.\n+ * Returns 0 on success and fills in err on failure.\n  */\n-int pack_refs(unsigned int flags);\n+int pack_refs(unsigned int flags, struct strbuf *err);\n \n extern int ref_exists(const char *);\n \n-- \n2.0.1.523.g70700c9\n"},{"id":"247480","messageId":"CAL=YDWn1uNvdKxdEg_D-=1-wvAm1PzbSM0N_G6_KUbuc7X7-Rw@mail.gmail.com","threadId":"37266","inReplyTo":"1406841910-3278-1-git-send-email-sahlberg@google.com","subject":"Re: [PATCH 0/5] ref-transactions-req-strbuf-err","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-08-08T16:15:29Z","receivedAt":"2014-08-08T16:15:29Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Ping ?\n\nOn Thu, Jul 31, 2014 at 2:25 PM, Ronnie Sahlberg <sahlberg@google.com> wrote:\n> List,\n>\n> This is the next patch series in the ref transaction work.\n> This patch series is called ref-transactions-req-strbuf-err and builds ontop\n> of the series called ref-transactions-req-packed-refs which is origin/pu\n>\n>\n> This patch series mainly adds some nice strbuf arguments to some functions to\n> pass errors back to callers.\n> The only thing noteworthy is that we finally get to remove\n> -enum action_on_err {\n> -       UPDATE_REFS_MSG_ON_ERR,\n> -       UPDATE_REFS_DIE_ON_ERR,\n> -       UPDATE_REFS_QUIET_ON_ERR\n> -};\n>\n> aside from that there is little/nothing much interesting in there.\n>\n>\n> Ronnie Sahlberg (5):\n>   refs.c: replace the onerr argument in update_ref with a strbuf err\n>   refs.c: make add_packed_ref return an error instead of calling die\n>   refs.c: make lock_packed_refs take an err argument\n>   refs.c: add an err argument to commit_packed_refs\n>   refs.c: add an err argument to pack_refs\n>\n>  builtin/checkout.c   |   7 ++-\n>  builtin/clone.c      |  23 +++++---\n>  builtin/merge.c      |  20 ++++---\n>  builtin/notes.c      |  24 +++++----\n>  builtin/pack-refs.c  |   8 ++-\n>  builtin/reset.c      |  12 +++--\n>  builtin/update-ref.c |   7 ++-\n>  notes-cache.c        |   2 +-\n>  notes-utils.c        |   5 +-\n>  refs.c               | 148 +++++++++++++++++++++++++++++----------------------\n>  refs.h               |  13 ++---\n>  transport-helper.c   |   7 ++-\n>  transport.c          |   9 ++--\n>  13 files changed, 170 insertions(+), 115 deletions(-)\n>\n> --\n> 2.0.1.523.g70700c9\n>\n"}]}