{"thread":{"id":"37900","subject":"[PATCH v3 0/7] ref-transaction-send-pack","startedAt":"2014-11-07T19:41:54Z","lastAt":"2014-11-07T19:42:01Z","messageCount":8,"participants":["Ronnie Sahlberg"],"isPatch":true,"patchVersion":3,"patchTotal":7},"messages":[{"id":"251492","messageId":"1415389321-10386-1-git-send-email-sahlberg@google.com","threadId":"37900","inReplyTo":null,"subject":"[PATCH v3 0/7] ref-transaction-send-pack","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-11-07T19:41:54Z","receivedAt":"2014-11-07T19:41:54Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"List,\n\nThis series has been posted before but is now rebased on the previous\nref-transaction-rename series that are against next.\nThis series can also be found at :\nhttps://github.com/rsahlberg/git/tree/ref-transactions-send-pack\n\nThis series finishes the transaction work to provide atomic pushes.\nWith this series we can now perform atomic pushes to a repository.\n\nVersion 2:\n- Reordered the capabilities we send so that agent= remains the last\n  capability listed.\n- Reworded the paragraph for atomic push in git-send-pack.txt\n- Dropped the patch for receive.preferatomicpush\n\nVersion 3:\n- Fix a typo in a commit message.\n\nRonnie Sahlberg (7):\n  receive-pack.c: add protocol support to negotiate atomic-push\n  send-pack.c: add an --atomic-push command line argument\n  receive-pack.c: use a single transaction when atomic-push is\n    negotiated\n  push.c: add an --atomic-push argument\n  t5543-atomic-push.sh: add basic tests for atomic pushes\n  refs.c: add an err argument to create_reflog\n  refs.c: add an err argument to create_symref\n\n Documentation/git-push.txt                        |   7 +-\n Documentation/git-send-pack.txt                   |   7 +-\n Documentation/technical/protocol-capabilities.txt |  12 ++-\n builtin/branch.c                                  |   7 +-\n builtin/checkout.c                                |  21 +++--\n builtin/clone.c                                   |  15 +++-\n builtin/init-db.c                                 |   8 +-\n builtin/notes.c                                   |   7 +-\n builtin/push.c                                    |   2 +\n builtin/receive-pack.c                            |  79 +++++++++++++----\n builtin/remote.c                                  |  26 ++++--\n builtin/send-pack.c                               |   6 +-\n builtin/symbolic-ref.c                            |   6 +-\n cache.h                                           |   1 -\n refs.c                                            |  93 ++++++++++----------\n refs.h                                            |   5 +-\n remote.h                                          |   3 +-\n send-pack.c                                       |  45 ++++++++--\n send-pack.h                                       |   1 +\n t/t5543-atomic-push.sh                            | 101 ++++++++++++++++++++++\n transport.c                                       |   5 ++\n transport.h                                       |   1 +\n 22 files changed, 358 insertions(+), 100 deletions(-)\n create mode 100755 t/t5543-atomic-push.sh\n\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"251500","messageId":"1415389321-10386-2-git-send-email-sahlberg@google.com","threadId":"37900","inReplyTo":"1415389321-10386-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 1/7] receive-pack.c: add protocol support to negotiate atomic-push","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-11-07T19:41:55Z","receivedAt":"2014-11-07T19:41:55Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"This adds support to the protocol between send-pack and receive-pack to\n* allow receive-pack to inform the client that it has atomic push capability\n* allow send-pack to request atomic push back.\n\nThere is currently no setting in send-pack to actually request that atomic\npushes are to be used yet. This only adds protocol capability not ability\nfor the user to activate it.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n Documentation/technical/protocol-capabilities.txt | 12 ++++++++++--\n builtin/receive-pack.c                            |  6 +++++-\n send-pack.c                                       |  6 ++++++\n 3 files changed, 21 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex 0c92dee..26bc5b1 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -18,8 +18,9 @@ was sent.  Server MUST NOT ignore capabilities that client requested\n and server advertised.  As a consequence of these rules, server MUST\n NOT advertise capabilities it does not understand.\n \n-The 'report-status', 'delete-refs', 'quiet', and 'push-cert' capabilities\n-are sent and recognized by the receive-pack (push to server) process.\n+The 'atomic-push', 'report-status', 'delete-refs', 'quiet', and 'push-cert'\n+capabilities are sent and recognized by the receive-pack (push to server)\n+process.\n \n The 'ofs-delta' and 'side-band-64k' capabilities are sent and recognized\n by both upload-pack and receive-pack protocols.  The 'agent' capability\n@@ -244,6 +245,13 @@ respond with the 'quiet' capability to suppress server-side progress\n reporting if the local progress reporting is also being suppressed\n (e.g., via `push -q`, or if stderr does not go to a tty).\n \n+atomic-push\n+-----------\n+\n+If the server sends the 'atomic-push' capability, it means it is\n+capable of accepting atomic pushes. If the pushing client requests this\n+capability, the server will update the refs in one single atomic transaction.\n+\n allow-tip-sha1-in-want\n ----------------------\n \ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex ccea9dc..65d9a7e 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -40,6 +40,7 @@ static int transfer_unpack_limit = -1;\n static int unpack_limit = 100;\n static int report_status;\n static int use_sideband;\n+static int use_atomic_push;\n static int quiet;\n static int prefer_ofs_delta = 1;\n static int auto_update_server_info;\n@@ -171,7 +172,8 @@ static void show_ref(const char *path, const unsigned char *sha1)\n \t\tstruct strbuf cap = STRBUF_INIT;\n \n \t\tstrbuf_addstr(&cap,\n-\t\t\t      \"report-status delete-refs side-band-64k quiet\");\n+\t\t\t      \"report-status delete-refs side-band-64k quiet \"\n+\t\t\t      \"atomic-push\");\n \t\tif (prefer_ofs_delta)\n \t\t\tstrbuf_addstr(&cap, \" ofs-delta\");\n \t\tif (push_cert_nonce)\n@@ -1178,6 +1180,8 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \t\t\t\tuse_sideband = LARGE_PACKET_MAX;\n \t\t\tif (parse_feature_request(feature_list, \"quiet\"))\n \t\t\t\tquiet = 1;\n+\t\t\tif (parse_feature_request(feature_list, \"atomic-push\"))\n+\t\t\t\tuse_atomic_push = 1;\n \t\t}\n \n \t\tif (!strcmp(line, \"push-cert\")) {\ndiff --git a/send-pack.c b/send-pack.c\nindex 949cb61..1ccc84c 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -294,6 +294,8 @@ int send_pack(struct send_pack_args *args,\n \tint use_sideband = 0;\n \tint quiet_supported = 0;\n \tint agent_supported = 0;\n+\tint atomic_push_supported = 0;\n+\tint atomic_push = 0;\n \tunsigned cmds_sent = 0;\n \tint ret;\n \tstruct async demux;\n@@ -314,6 +316,8 @@ int send_pack(struct send_pack_args *args,\n \t\tagent_supported = 1;\n \tif (server_supports(\"no-thin\"))\n \t\targs->use_thin_pack = 0;\n+\tif (server_supports(\"atomic-push\"))\n+\t\tatomic_push_supported = 1;\n \tif (args->push_cert) {\n \t\tint len;\n \n@@ -335,6 +339,8 @@ int send_pack(struct send_pack_args *args,\n \t\tstrbuf_addstr(&cap_buf, \" side-band-64k\");\n \tif (quiet_supported && (args->quiet || !args->progress))\n \t\tstrbuf_addstr(&cap_buf, \" quiet\");\n+\tif (atomic_push)\n+\t\tstrbuf_addstr(&cap_buf, \" atomic-push\");\n \tif (agent_supported)\n \t\tstrbuf_addf(&cap_buf, \" agent=%s\", git_user_agent_sanitized());\n \n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"251493","messageId":"1415389321-10386-3-git-send-email-sahlberg@google.com","threadId":"37900","inReplyTo":"1415389321-10386-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 2/7] send-pack.c: add an --atomic-push command line argument","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-11-07T19:41:56Z","receivedAt":"2014-11-07T19:41:56Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"This adds support to send-pack to negotiate and use atomic pushes\niff the server supports it. Atomic pushes are activated by a new command\nline flag --atomic-push.\n\nIn order to do this we also need to change the semantics for send_pack()\nslightly. The existing send_pack() function actually don't sent all the\nrefs back to the server when multiple refs are involved, for example\nwhen using --all. Several of the failure modes for pushes can already be\ndetected locally in the send_pack client based on the information from the\ninitial server side list of all the refs as generated by receive-pack.\nAny such refs that we thus know would fail to push are thus pruned from\nthe list of refs we send to the server to update.\n\nFor atomic pushes, we have to deal thus with both failures that are detected\nlocally as well as failures that are reported back from the server. In order\nto do so we treat all local failures as push failures too.\n\nWe introduce a new status code REF_STATUS_ATOMIC_PUSH_FAILED so we can\nflag all refs that we would normally have tried to push to the server\nbut we did not due to local failures. This is to improve the error message\nback to the end user to flag that \"these refs failed to update since the\natomic push operation failed.\"\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n Documentation/git-send-pack.txt |  7 ++++++-\n builtin/send-pack.c             |  6 +++++-\n remote.h                        |  3 ++-\n send-pack.c                     | 39 ++++++++++++++++++++++++++++++++++-----\n send-pack.h                     |  1 +\n transport.c                     |  4 ++++\n 6 files changed, 52 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt\nindex 2a0de42..9296587 100644\n--- a/Documentation/git-send-pack.txt\n+++ b/Documentation/git-send-pack.txt\n@@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another repository\n SYNOPSIS\n --------\n [verse]\n-'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n+'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]\n \n DESCRIPTION\n -----------\n@@ -62,6 +62,11 @@ be in a separate packet, and the list must end with a flush packet.\n \tSend a \"thin\" pack, which records objects in deltified form based\n \ton objects not included in the pack to reduce network traffic.\n \n+--atomic-push::\n+\tUse an atomic transaction for updating the refs. If any of the refs\n+\tfails to update then the entire push will fail without changing any\n+\trefs.\n+\n <host>::\n \tA remote host to house the repository.  When this\n \tpart is specified, 'git-receive-pack' is invoked via\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex b564a77..93cb17c 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -13,7 +13,7 @@\n #include \"sha1-array.h\"\n \n static const char send_pack_usage[] =\n-\"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\\n\"\n+\"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]\\n\"\n \"  --all and explicit <ref> specification are mutually exclusive.\";\n \n static struct send_pack_args args;\n@@ -170,6 +170,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\targs.use_thin_pack = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--atomic-push\")) {\n+\t\t\t\targs.use_atomic_push = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--stateless-rpc\")) {\n \t\t\t\targs.stateless_rpc = 1;\n \t\t\t\tcontinue;\ndiff --git a/remote.h b/remote.h\nindex 8b62efd..f346524 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -115,7 +115,8 @@ struct ref {\n \t\tREF_STATUS_REJECT_SHALLOW,\n \t\tREF_STATUS_UPTODATE,\n \t\tREF_STATUS_REMOTE_REJECT,\n-\t\tREF_STATUS_EXPECTING_REPORT\n+\t\tREF_STATUS_EXPECTING_REPORT,\n+\t\tREF_STATUS_ATOMIC_PUSH_FAILED\n \t} status;\n \tchar *remote_status;\n \tstruct ref *peer_ref; /* when renaming */\ndiff --git a/send-pack.c b/send-pack.c\nindex 1ccc84c..08602a8 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -190,7 +190,7 @@ static void advertise_shallow_grafts_buf(struct strbuf *sb)\n \tfor_each_commit_graft(advertise_shallow_grafts_cb, sb);\n }\n \n-static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args)\n+static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args, int *atomic_push_failed)\n {\n \tif (!ref->peer_ref && !args->send_mirror)\n \t\treturn 0;\n@@ -203,6 +203,12 @@ static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_a\n \tcase REF_STATUS_REJECT_NEEDS_FORCE:\n \tcase REF_STATUS_REJECT_STALE:\n \tcase REF_STATUS_REJECT_NODELETE:\n+\t\tif (atomic_push_failed) {\n+\t\t\tfprintf(stderr, \"Atomic push failed for ref %s. \"\n+\t\t\t\t\"Status:%d\\n\", ref->name, ref->status);\n+\t\t\t*atomic_push_failed = 1;\n+\t\t}\n+\t\t/* fallthrough */\n \tcase REF_STATUS_UPTODATE:\n \t\treturn 0;\n \tdefault:\n@@ -250,7 +256,7 @@ static int generate_push_cert(struct strbuf *req_buf,\n \tstrbuf_addstr(&cert, \"\\n\");\n \n \tfor (ref = remote_refs; ref; ref = ref->next) {\n-\t\tif (!ref_update_to_be_sent(ref, args))\n+\t\tif (!ref_update_to_be_sent(ref, args, NULL))\n \t\t\tcontinue;\n \t\tupdate_seen = 1;\n \t\tstrbuf_addf(&cert, \"%s %s %s\\n\",\n@@ -297,7 +303,7 @@ int send_pack(struct send_pack_args *args,\n \tint atomic_push_supported = 0;\n \tint atomic_push = 0;\n \tunsigned cmds_sent = 0;\n-\tint ret;\n+\tint ret, atomic_push_failed = 0;\n \tstruct async demux;\n \tconst char *push_cert_nonce = NULL;\n \n@@ -332,6 +338,11 @@ int send_pack(struct send_pack_args *args,\n \t\t\t\"Perhaps you should specify a branch such as 'master'.\\n\");\n \t\treturn 0;\n \t}\n+\tif (args->use_atomic_push && !atomic_push_supported) {\n+\t\tfprintf(stderr, \"Server does not support atomic-push.\");\n+\t\treturn -1;\n+\t}\n+\tatomic_push = atomic_push_supported && args->use_atomic_push;\n \n \tif (status_report)\n \t\tstrbuf_addstr(&cap_buf, \" report-status\");\n@@ -365,7 +376,8 @@ int send_pack(struct send_pack_args *args,\n \t * the pack data.\n \t */\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n-\t\tif (!ref_update_to_be_sent(ref, args))\n+\t\tif (!ref_update_to_be_sent(ref, args,\n+\t\t\targs->use_atomic_push ? &atomic_push_failed : NULL))\n \t\t\tcontinue;\n \n \t\tif (!ref->deletion)\n@@ -377,6 +389,23 @@ int send_pack(struct send_pack_args *args,\n \t\t\tref->status = REF_STATUS_EXPECTING_REPORT;\n \t}\n \n+\tif (atomic_push_failed) {\n+\t\tfor (ref = remote_refs; ref; ref = ref->next) {\n+\t\t\tif (!ref->peer_ref && !args->send_mirror)\n+\t\t\t\tcontinue;\n+\n+\t\t\tswitch (ref->status) {\n+\t\t\tcase REF_STATUS_EXPECTING_REPORT:\n+\t\t\t\tref->status = REF_STATUS_ATOMIC_PUSH_FAILED;\n+\t\t\t\tcontinue;\n+\t\t\tdefault:\n+\t\t\t\t; /* do nothing */\n+\t\t\t}\n+\t\t}\n+\t\tfprintf(stderr, \"Atomic push failed.\");\n+\t\treturn -1;\n+\t}\n+\n \t/*\n \t * Finally, tell the other end!\n \t */\n@@ -386,7 +415,7 @@ int send_pack(struct send_pack_args *args,\n \t\tif (args->dry_run || args->push_cert)\n \t\t\tcontinue;\n \n-\t\tif (!ref_update_to_be_sent(ref, args))\n+\t\tif (!ref_update_to_be_sent(ref, args, NULL))\n \t\t\tcontinue;\n \n \t\told_hex = sha1_to_hex(ref->old_sha1);\ndiff --git a/send-pack.h b/send-pack.h\nindex 5635457..7486e65 100644\n--- a/send-pack.h\n+++ b/send-pack.h\n@@ -11,6 +11,7 @@ struct send_pack_args {\n \t\tforce_update:1,\n \t\tuse_thin_pack:1,\n \t\tuse_ofs_delta:1,\n+\t\tuse_atomic_push:1,\n \t\tdry_run:1,\n \t\tpush_cert:1,\n \t\tstateless_rpc:1;\ndiff --git a/transport.c b/transport.c\nindex f70d62f..2111986 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -731,6 +731,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i\n \t\t\t\t\t\t ref->deletion ? NULL : ref->peer_ref,\n \t\t\t\t\t\t \"remote failed to report status\", porcelain);\n \t\tbreak;\n+\tcase REF_STATUS_ATOMIC_PUSH_FAILED:\n+\t\tprint_ref_status('!', \"[rejected]\", ref, ref->peer_ref,\n+\t\t\t\t\t\t \"atomic-push-failed\", porcelain);\n+\t\tbreak;\n \tcase REF_STATUS_OK:\n \t\tprint_ok_ref_status(ref, porcelain);\n \t\tbreak;\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"251497","messageId":"1415389321-10386-4-git-send-email-sahlberg@google.com","threadId":"37900","inReplyTo":"1415389321-10386-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 3/7] receive-pack.c: use a single transaction when atomic-push is negotiated","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-11-07T19:41:57Z","receivedAt":"2014-11-07T19:41:57Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Update receive-pack to use an atomic transaction iff the client negotiated\nthat it wanted atomic-push.\nThis leaves the default behavior to be the old non-atomic one ref at a\ntime update. This is to cause as little disruption as possible to existing\nclients. It is unknown if there are client scripts that depend on the old\nnon-atomic behavior so we make it opt-in for now.\n\nLater patch in this series also adds a configuration variable where you can\noverride the atomic push behavior on the receiving repo and force it\nto use atomic updates always.\n\nIf it turns out over time that there are no client scripts that depend on the\nold behavior we can change git to default to use atomic pushes and instead\noffer an opt-out argument for people that do not want atomic pushes.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/receive-pack.c | 73 +++++++++++++++++++++++++++++++++++++++-----------\n 1 file changed, 58 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 65d9a7e..27b49dd 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -67,6 +67,8 @@ static const char *NONCE_SLOP = \"SLOP\";\n static const char *nonce_status;\n static long nonce_stamp_slop;\n static unsigned long nonce_stamp_slop_limit;\n+struct strbuf err = STRBUF_INIT;\n+struct transaction *transaction;\n \n static enum deny_action parse_deny_action(const char *var, const char *value)\n {\n@@ -832,33 +834,55 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t\t\tcmd->did_not_exist = 1;\n \t\t\t}\n \t\t}\n-\t\tif (delete_ref(namespaced_name, old_sha1, 0)) {\n-\t\t\trp_error(\"failed to delete %s\", name);\n-\t\t\treturn \"failed to delete\";\n+\t\tif (!use_atomic_push) {\n+\t\t\tif (delete_ref(namespaced_name, old_sha1, 0)) {\n+\t\t\t\trp_error(\"failed to delete %s\", name);\n+\t\t\t\treturn \"failed to delete\";\n+\t\t\t}\n+\t\t} else {\n+\t\t\tif (transaction_delete_ref(transaction,\n+\t\t\t\t\t\t   namespaced_name,\n+\t\t\t\t\t\t   old_sha1,\n+\t\t\t\t\t\t   0, old_sha1 != NULL,\n+\t\t\t\t\t\t   \"push\", &err)) {\n+\t\t\t\trp_error(\"%s\", err.buf);\n+\t\t\t\tstrbuf_release(&err);\n+\t\t\t\treturn \"failed to delete\";\n+\t\t\t}\n \t\t}\n \t\treturn NULL; /* good */\n \t}\n \telse {\n-\t\tstruct strbuf err = STRBUF_INIT;\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 = transaction_begin(&err);\n-\t\tif (!transaction ||\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    transaction_commit(transaction, &err)) {\n-\t\t\ttransaction_free(transaction);\n+\t\tif (!use_atomic_push) {\n+\t\t\ttransaction = transaction_begin(&err);\n+\t\t\tif (!transaction) {\n+\t\t\t\trp_error(\"%s\", err.buf);\n+\t\t\t\tstrbuf_release(&err);\n+\t\t\t\treturn \"failed to start transaction\";\n+\t\t\t}\n+\t\t}\n+\t\tif (transaction_update_ref(transaction,\n+\t\t\t\t\t   namespaced_name,\n+\t\t\t\t\t   new_sha1, old_sha1,\n+\t\t\t\t\t   0, 1, \"push\",\n+\t\t\t\t\t   &err)) {\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\ttransaction_free(transaction);\n+\t\tif (!use_atomic_push) {\n+\t\t\tif (transaction_commit(transaction, &err)) {\n+\t\t\t\ttransaction_free(transaction);\n+\t\t\t\trp_error(\"%s\", err.buf);\n+\t\t\t\tstrbuf_release(&err);\n+\t\t\t\treturn \"failed to update ref\";\n+\t\t\t}\n+\t\t\ttransaction_free(transaction);\n+\t\t}\n \t\tstrbuf_release(&err);\n \t\treturn NULL; /* good */\n \t}\n@@ -1058,6 +1082,16 @@ static void execute_commands(struct command *commands,\n \t\treturn;\n \t}\n \n+\tif (use_atomic_push) {\n+\t\ttransaction = transaction_begin(&err);\n+\t\tif (!transaction) {\n+\t\t\terror(\"%s\", err.buf);\n+\t\t\tstrbuf_release(&err);\n+\t\t\tfor (cmd = commands; cmd; cmd = cmd->next)\n+\t\t\t\tcmd->error_string = \"transaction error\";\n+\t\t\treturn;\n+\t\t}\n+\t}\n \tdata.cmds = commands;\n \tdata.si = si;\n \tif (check_everything_connected(iterate_receive_command_list, 0, &data))\n@@ -1095,6 +1129,14 @@ static void execute_commands(struct command *commands,\n \t\t}\n \t}\n \n+\tif (use_atomic_push) {\n+\t\tif (transaction_commit(transaction, &err)) {\n+\t\t\trp_error(\"%s\", err.buf);\n+\t\t\tfor (cmd = commands; cmd; cmd = cmd->next)\n+\t\t\t\tcmd->error_string = err.buf;\n+\t\t}\n+\t\ttransaction_free(transaction);\n+\t}\n \tif (shallow_update && !checked_connectivity)\n \t\terror(\"BUG: run 'git fsck' for safety.\\n\"\n \t\t      \"If there are errors, try to remove \"\n@@ -1542,5 +1584,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \tsha1_array_clear(&shallow);\n \tsha1_array_clear(&ref);\n \tfree((void *)push_cert_nonce);\n+\tstrbuf_release(&err);\n \treturn 0;\n }\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"251499","messageId":"1415389321-10386-5-git-send-email-sahlberg@google.com","threadId":"37900","inReplyTo":"1415389321-10386-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 4/7] push.c: add an --atomic-push argument","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-11-07T19:41:58Z","receivedAt":"2014-11-07T19:41:58Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Add a command line argument to the git push command to request atomic\npushes.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n Documentation/git-push.txt | 7 ++++++-\n builtin/push.c             | 2 ++\n transport.c                | 1 +\n transport.h                | 1 +\n 4 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 21b3f29..04de8d8 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -9,7 +9,7 @@ git-push - Update remote refs along with associated objects\n SYNOPSIS\n --------\n [verse]\n-'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=<git-receive-pack>]\n+'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic-push] [-n | --dry-run] [--receive-pack=<git-receive-pack>]\n \t   [--repo=<repository>] [-f | --force] [--prune] [-v | --verbose]\n \t   [-u | --set-upstream] [--signed]\n \t   [--force-with-lease[=<refname>[:<expect>]]]\n@@ -136,6 +136,11 @@ already exists on the remote side.\n \tlogged.  See linkgit:git-receive-pack[1] for the details\n \ton the receiving end.\n \n+--atomic-push::\n+\tTry using atomic push. If atomic push is negotiated with the server\n+\tthen any push covering multiple refs will be atomic. Either all\n+\trefs are updated, or on error, no refs are updated.\n+\n --receive-pack=<git-receive-pack>::\n --exec=<git-receive-pack>::\n \tPath to the 'git-receive-pack' program on the remote\ndiff --git a/builtin/push.c b/builtin/push.c\nindex ae56f73..0b9f21a 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -507,6 +507,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT(0, \"follow-tags\", &flags, N_(\"push missing but relevant tags\"),\n \t\t\tTRANSPORT_PUSH_FOLLOW_TAGS),\n \t\tOPT_BIT(0, \"signed\", &flags, N_(\"GPG sign the push\"), TRANSPORT_PUSH_CERT),\n+\t\tOPT_BIT(0, \"atomic-push\", &flags, N_(\"use atomic push, if available\"),\n+\t\t\tTRANSPORT_ATOMIC_PUSH),\n \t\tOPT_END()\n \t};\n \ndiff --git a/transport.c b/transport.c\nindex 2111986..cd2b63a 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -833,6 +833,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n \targs.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);\n \targs.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);\n \targs.push_cert = !!(flags & TRANSPORT_PUSH_CERT);\n+\targs.use_atomic_push = !!(flags & TRANSPORT_ATOMIC_PUSH);\n \targs.url = transport->url;\n \n \tret = send_pack(&args, data->fd, data->conn, remote_refs,\ndiff --git a/transport.h b/transport.h\nindex 3e0091e..25fa1da 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -125,6 +125,7 @@ struct transport {\n #define TRANSPORT_PUSH_NO_HOOK 512\n #define TRANSPORT_PUSH_FOLLOW_TAGS 1024\n #define TRANSPORT_PUSH_CERT 2048\n+#define TRANSPORT_ATOMIC_PUSH 4096\n \n #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n #define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"251494","messageId":"1415389321-10386-6-git-send-email-sahlberg@google.com","threadId":"37900","inReplyTo":"1415389321-10386-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 5/7] t5543-atomic-push.sh: add basic tests for atomic pushes","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-11-07T19:41:59Z","receivedAt":"2014-11-07T19:41:59Z","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 t/t5543-atomic-push.sh | 101 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 101 insertions(+)\n create mode 100755 t/t5543-atomic-push.sh\n\ndiff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh\nnew file mode 100755\nindex 0000000..4903227\n--- /dev/null\n+++ b/t/t5543-atomic-push.sh\n@@ -0,0 +1,101 @@\n+#!/bin/sh\n+\n+test_description='pushing to a mirror repository'\n+\n+. ./test-lib.sh\n+\n+D=`pwd`\n+\n+invert () {\n+\tif \"$@\"; then\n+\t\treturn 1\n+\telse\n+\t\treturn 0\n+\tfi\n+}\n+\n+mk_repo_pair () {\n+\trm -rf master mirror &&\n+\tmkdir mirror &&\n+\t(\n+\t\tcd mirror &&\n+\t\tgit init &&\n+\t\tgit config receive.denyCurrentBranch warn\n+\t) &&\n+\tmkdir master &&\n+\t(\n+\t\tcd master &&\n+\t\tgit init &&\n+\t\tgit remote add $1 up ../mirror\n+\t)\n+}\n+\n+\n+test_expect_success 'atomic push works for a single branch' '\n+\n+\tmk_repo_pair &&\n+\t(\n+\t\tcd master &&\n+\t\techo one >foo && git add foo && git commit -m one &&\n+\t\tgit push --mirror up\n+\t\techo two >foo && git add foo && git commit -m two &&\n+\t\tgit push --atomic-push --mirror up\n+\t) &&\n+\tmaster_master=$(cd master && git show-ref -s --verify refs/heads/master) &&\n+\tmirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&\n+\ttest \"$master_master\" = \"$mirror_master\"\n+\n+'\n+\n+test_expect_success 'atomic push works for two branches' '\n+\n+\tmk_repo_pair &&\n+\t(\n+\t\tcd master &&\n+\t\techo one >foo && git add foo && git commit -m one &&\n+\t\tgit branch second &&\n+\t\tgit push --mirror up\n+\t\techo two >foo && git add foo && git commit -m two &&\n+\t\tgit checkout second &&\n+\t\techo three >foo && git add foo && git commit -m three &&\n+\t\tgit checkout master &&\n+\t\tgit push --atomic-push --mirror up\n+\t) &&\n+\tmaster_master=$(cd master && git show-ref -s --verify refs/heads/master) &&\n+\tmirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&\n+\ttest \"$master_master\" = \"$mirror_master\"\n+\n+\tmaster_second=$(cd master && git show-ref -s --verify refs/heads/second) &&\n+\tmirror_second=$(cd mirror && git show-ref -s --verify refs/heads/second) &&\n+\ttest \"$master_second\" = \"$mirror_second\"\n+'\n+\n+# set up two branches where master can be pushed but second can not\n+# (non-fast-forward). Since second can not be pushed the whole operation\n+# will fail and leave master untouched.\n+test_expect_success 'atomic push fails if one branch fails' '\n+\tmk_repo_pair &&\n+\t(\n+\t\tcd master &&\n+\t\techo one >foo && git add foo && git commit -m one &&\n+\t\tgit branch second &&\n+\t\tgit checkout second &&\n+\t\techo two >foo && git add foo && git commit -m two &&\n+\t\techo three >foo && git add foo && git commit -m three &&\n+\t\techo four >foo && git add foo && git commit -m four &&\n+\t\tgit push --mirror up\n+\t\tgit reset --hard HEAD~2 &&\n+\t\tgit checkout master\n+\t\techo five >foo && git add foo && git commit -m five &&\n+\t\t! git push --atomic-push --all up\n+\t) &&\n+\tmaster_master=$(cd master && git show-ref -s --verify refs/heads/master) &&\n+\tmirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&\n+\ttest \"$master_master\" != \"$mirror_master\" &&\n+\n+\tmaster_second=$(cd master && git show-ref -s --verify refs/heads/second) &&\n+\tmirror_second=$(cd mirror && git show-ref -s --verify refs/heads/second) &&\n+\ttest \"$master_second\" != \"$mirror_second\"\n+'\n+\n+test_done\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"251495","messageId":"1415389321-10386-7-git-send-email-sahlberg@google.com","threadId":"37900","inReplyTo":"1415389321-10386-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 6/7] refs.c: add an err argument to create_reflog","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-11-07T19:42:00Z","receivedAt":"2014-11-07T19:42:00Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Add err argument to create_reflog that can explain the reason for a\nfailure. This then eliminates the need to manage errno through this\nfunction since we can just add strerror(errno) to the err string when\nmeaningful. No callers relied on errno from this function for anything\nelse than the error message.\n\nlog_ref_write is a private function that calls create_reflog. Update\nthis function to also take an err argument and pass it back to the caller.\nThis again eliminates the need to manage errno in this function.\n\nUpdate the private function write_sha1_update_reflog to also take an\nerr argument.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/checkout.c |  8 +++---\n refs.c             | 71 +++++++++++++++++++++++++++---------------------------\n refs.h             |  4 +--\n 3 files changed, 42 insertions(+), 41 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 60a68f7..d9cb9c3 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -590,10 +590,12 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n \t\tif (opts->new_orphan_branch) {\n \t\t\tif (opts->new_branch_log && !log_all_ref_updates) {\n \t\t\t\tchar *ref_name = mkpath(\"refs/heads/%s\", opts->new_orphan_branch);\n+\t\t\t\tstruct strbuf err = STRBUF_INIT;\n \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\tif (create_reflog(ref_name, &err)) {\n+\t\t\t\t\tfprintf(stderr, _(\"Can not do reflog for '%s'. %s\\n\"),\n+\t\t\t\t\topts->new_orphan_branch, err.buf);\n+\t\t\t\t\tstrbuf_release(&err);\n \t\t\t\t\treturn;\n \t\t\t\t}\n \t\t\t}\ndiff --git a/refs.c b/refs.c\nindex cafb4aa..fc9ace2 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2895,8 +2895,7 @@ static int copy_msg(char *buf, const char *msg)\n \treturn cp - buf;\n }\n \n-/* This function must set a meaningful errno on failure */\n-int create_reflog(const char *refname)\n+int create_reflog(const char *refname, struct strbuf *err)\n {\n \tint logfd, oflags = O_APPEND | O_WRONLY;\n \tchar logfile[PATH_MAX];\n@@ -2907,9 +2906,8 @@ int create_reflog(const char *refname)\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-\t\t\terrno = save_errno;\n+\t\t\tstrbuf_addf(err, \"unable to create directory for %s. \"\n+\t\t\t\t    \"%s\", logfile, strerror(errno));\n \t\t\treturn -1;\n \t\t}\n \t\toflags |= O_CREAT;\n@@ -2922,20 +2920,16 @@ int create_reflog(const char *refname)\n \n \t\tif ((oflags & O_CREAT) && errno == EISDIR) {\n \t\t\tif (remove_empty_directories(logfile)) {\n-\t\t\t\tint save_errno = errno;\n-\t\t\t\terror(\"There are still logs under '%s'\",\n-\t\t\t\t      logfile);\n-\t\t\t\terrno = save_errno;\n+\t\t\t\tstrbuf_addf(err, \"There are still logs under \"\n+\t\t\t\t\t    \"'%s'\", logfile);\n \t\t\t\treturn -1;\n \t\t\t}\n \t\t\tlogfd = open(logfile, oflags, 0666);\n \t\t}\n \n \t\tif (logfd < 0) {\n-\t\t\tint save_errno = errno;\n-\t\t\terror(\"Unable to append to %s: %s\", logfile,\n-\t\t\t      strerror(errno));\n-\t\t\terrno = save_errno;\n+\t\t\tstrbuf_addf(err, \"Unable to append to %s: %s\",\n+\t\t\t\t    logfile, strerror(errno));\n \t\t\treturn -1;\n \t\t}\n \t}\n@@ -2972,7 +2966,8 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,\n }\n \n static int log_ref_write(const char *refname, const unsigned char *old_sha1,\n-\t\t\t const unsigned char *new_sha1, const char *msg)\n+\t\t\t const unsigned char *new_sha1, const char *msg,\n+\t\t\t struct strbuf *err)\n {\n \tint logfd, result = 0, oflags = O_APPEND | O_WRONLY;\n \tchar log_file[PATH_MAX];\n@@ -2981,7 +2976,7 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,\n \t\tlog_all_ref_updates = !is_bare_repository();\n \n \tif (log_all_ref_updates && !reflog_exists(refname))\n-\t\tresult = create_reflog(refname);\n+\t\tresult = create_reflog(refname, err);\n \n \tif (result)\n \t\treturn result;\n@@ -2994,16 +2989,14 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,\n \tresult = log_ref_write_fd(logfd, old_sha1, new_sha1,\n \t\t\t\t  git_committer_info(0), msg);\n \tif (result) {\n-\t\tint save_errno = errno;\n \t\tclose(logfd);\n-\t\terror(\"Unable to append to %s\", log_file);\n-\t\terrno = save_errno;\n+\t\tstrbuf_addf(err, \"Unable to append to %s. %s\", log_file,\n+\t\t\t    strerror(errno));\n \t\treturn -1;\n \t}\n \tif (close(logfd)) {\n-\t\tint save_errno = errno;\n-\t\terror(\"Unable to append to %s\", log_file);\n-\t\terrno = save_errno;\n+\t\tstrbuf_addf(err, \"Unable to append to %s. %s\", log_file,\n+\t\t\t    strerror(errno));\n \t\treturn -1;\n \t}\n \treturn 0;\n@@ -3015,12 +3008,12 @@ int is_branch(const char *refname)\n }\n \n static int write_sha1_update_reflog(struct ref_lock *lock,\n-\tconst unsigned char *sha1, const char *logmsg)\n+\t\t\t\t    const unsigned char *sha1,\n+\t\t\t\t    const char *logmsg, struct strbuf *err)\n {\n-\tif (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||\n+\tif (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg, err) < 0 ||\n \t    (strcmp(lock->ref_name, lock->orig_ref_name) &&\n-\t     log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {\n-\t\tunlock_ref(lock);\n+\t     log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg, err) < 0)) {\n \t\treturn -1;\n \t}\n \tif (strcmp(lock->orig_ref_name, \"HEAD\") != 0) {\n@@ -3042,8 +3035,11 @@ static int write_sha1_update_reflog(struct ref_lock *lock,\n \t\thead_ref = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n \t\t\t\t\t      head_sha1, &head_flag);\n \t\tif (head_ref && (head_flag & REF_ISSYMREF) &&\n-\t\t    !strcmp(head_ref, lock->ref_name))\n-\t\t\tlog_ref_write(\"HEAD\", lock->old_sha1, sha1, logmsg);\n+\t\t    !strcmp(head_ref, lock->ref_name) &&\n+\t\t    log_ref_write(\"HEAD\", lock->old_sha1, sha1, logmsg, err)) {\n+\t\t\terror(\"%s\", err->buf);\n+\t\t\tstrbuf_release(err);\n+\t\t}\n \t}\n \treturn 0;\n }\n@@ -3057,6 +3053,7 @@ static int write_ref_sha1(struct ref_lock *lock,\n {\n \tstatic char term = '\\n';\n \tstruct object *o;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tif (!lock) {\n \t\terrno = EINVAL;\n@@ -3091,8 +3088,10 @@ static int write_ref_sha1(struct ref_lock *lock,\n \t\treturn -1;\n \t}\n \tclear_loose_ref_cache(&ref_cache);\n-\tif (write_sha1_update_reflog(lock, sha1, logmsg)) {\n+\tif (write_sha1_update_reflog(lock, sha1, logmsg, &err)) {\n \t\tunlock_ref(lock);\n+\t\terror(\"%s\", err.buf);\n+\t\tstrbuf_release(&err);\n \t\treturn -1;\n \t}\n \tif (commit_ref(lock)) {\n@@ -3112,6 +3111,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,\n \tint fd, len, written;\n \tchar *git_HEAD = git_pathdup(\"%s\", ref_target);\n \tunsigned char old_sha1[20], new_sha1[20];\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tif (logmsg && read_ref(ref_target, old_sha1))\n \t\thashclr(old_sha1);\n@@ -3160,9 +3160,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,\n #ifndef NO_SYMLINK_HEAD\n \tdone:\n #endif\n-\tif (logmsg && !read_ref(refs_heads_master, new_sha1))\n-\t\tlog_ref_write(ref_target, old_sha1, new_sha1, logmsg);\n-\n+\tif (logmsg && !read_ref(refs_heads_master, new_sha1) &&\n+\t    log_ref_write(ref_target, old_sha1, new_sha1, logmsg, &err))\n+\t\terror(\"%s\", err.buf);\n+\t\n+\tstrbuf_release(&err);\n \tfree(git_HEAD);\n \treturn 0;\n }\n@@ -3936,10 +3938,7 @@ int transaction_commit(struct transaction *transaction,\n \t\t\tgoto cleanup;\n \t\t}\n \t\tif (write_sha1_update_reflog(update->lock, update->new_sha1,\n-\t\t\t\t\t     update->msg)) {\n-\t\t\tif (err)\n-\t\t\t\tstrbuf_addf(err, \"Failed to update log '%s'.\",\n-\t\t\t\t\t    update->refname);\n+\t\t\t\t\t     update->msg, err)) {\n \t\t\tret = -1;\n \t\t\tgoto cleanup;\n \t\t}\n@@ -3976,7 +3975,7 @@ int transaction_commit(struct transaction *transaction,\n \t\t\tcontinue;\n \t\t}\n \t\tif (log_all_ref_updates && !reflog_exists(update->refname) &&\n-\t\t    create_reflog(update->refname)) {\n+\t\t    create_reflog(update->refname, err)) {\n \t\t\tret = -1;\n \t\t\tif (err)\n \t\t\t\tstrbuf_addf(err, \"Failed to setup reflog for \"\ndiff --git a/refs.h b/refs.h\nindex 489aa9d..930821e 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -170,8 +170,8 @@ 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+/** Create reflog. Fill in err on failure. */\n+extern int create_reflog(const char *refname, struct strbuf *err);\n \n /** Delete a reflog */\n extern int delete_reflog(const char *refname);\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"251496","messageId":"1415389321-10386-8-git-send-email-sahlberg@google.com","threadId":"37900","inReplyTo":"1415389321-10386-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 7/7] refs.c: add an err argument to create_symref","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-11-07T19:42:01Z","receivedAt":"2014-11-07T19:42:01Z","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/branch.c       |  7 +++++--\n builtin/checkout.c     | 13 ++++++++++---\n builtin/clone.c        | 15 +++++++++++----\n builtin/init-db.c      |  8 ++++++--\n builtin/notes.c        |  7 ++++---\n builtin/remote.c       | 26 ++++++++++++++++++--------\n builtin/symbolic-ref.c |  6 +++++-\n cache.h                |  1 -\n refs.c                 | 30 ++++++++++++++++++------------\n refs.h                 |  1 +\n 10 files changed, 78 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 04f57d4..ab6d9f4 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -698,6 +698,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)\n {\n \tstruct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;\n \tstruct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \tint recovery = 0;\n \tint clobber_head_ok;\n \n@@ -734,8 +735,10 @@ static void rename_branch(const char *oldname, const char *newname, int force)\n \t\twarning(_(\"Renamed a misnamed branch '%s' away\"), oldref.buf + 11);\n \n \t/* no need to pass logmsg here as HEAD didn't really move */\n-\tif (!strcmp(oldname, head) && create_symref(\"HEAD\", newref.buf, NULL))\n-\t\tdie(_(\"Branch renamed to %s, but HEAD is not updated!\"), newname);\n+\tif (!strcmp(oldname, head) &&\n+\t    create_symref(\"HEAD\", newref.buf, NULL, &err))\n+\t\tdie(_(\"Branch renamed to %s, but HEAD is not updated!. %s\"),\n+\t\t    newname, err.buf);\n \n \tstrbuf_addf(&oldsection, \"branch.%s\", oldref.buf + 11);\n \tstrbuf_release(&oldref);\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex d9cb9c3..1efe353 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -634,7 +634,10 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n \t\t\tdescribe_detached_head(_(\"HEAD is now at\"), new->commit);\n \t\t}\n \t} else if (new->path) {\t/* Switch branches. */\n-\t\tcreate_symref(\"HEAD\", new->path, msg.buf);\n+\t\tif (create_symref(\"HEAD\", new->path, msg.buf, &err)) {\n+\t\t\terror(\"%s\", err.buf);\n+\t\t\tstrbuf_release(&err);\n+\t\t}\n \t\tif (!opts->quiet) {\n \t\t\tif (old->path && !strcmp(new->path, old->path)) {\n \t\t\t\tif (opts->new_branch_force)\n@@ -1020,12 +1023,16 @@ static int parse_branchname_arg(int argc, const char **argv,\n static int switch_unborn_to_new_branch(const struct checkout_opts *opts)\n {\n \tint status;\n-\tstruct strbuf branch_ref = STRBUF_INIT;\n+\tstruct strbuf branch_ref = STRBUF_INIT, err = STRBUF_INIT;\n \n \tif (!opts->new_branch)\n \t\tdie(_(\"You are on a branch yet to be born\"));\n \tstrbuf_addf(&branch_ref, \"refs/heads/%s\", opts->new_branch);\n-\tstatus = create_symref(\"HEAD\", branch_ref.buf, \"checkout -b\");\n+\tstatus = create_symref(\"HEAD\", branch_ref.buf, \"checkout -b\", &err);\n+\tif (status) {\n+\t\terror(\"%s\", err.buf);\n+\t\tstrbuf_release(&err);\n+\t}\n \tstrbuf_release(&branch_ref);\n \tif (!opts->quiet)\n \t\tfprintf(stderr, _(\"Switched to a new branch '%s'\\n\"),\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 5577b5b..17b6ae8 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -565,6 +565,7 @@ static void update_remote_refs(const struct ref *refs,\n \t\t\t       int check_connectivity)\n {\n \tconst struct ref *rm = mapped_refs;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tif (check_connectivity) {\n \t\tif (transport->progress)\n@@ -586,9 +587,12 @@ static void update_remote_refs(const struct ref *refs,\n \t\tstruct strbuf head_ref = STRBUF_INIT;\n \t\tstrbuf_addstr(&head_ref, branch_top);\n \t\tstrbuf_addstr(&head_ref, \"HEAD\");\n-\t\tcreate_symref(head_ref.buf,\n-\t\t\t      remote_head_points_at->peer_ref->name,\n-\t\t\t      msg);\n+\t\tif (create_symref(head_ref.buf,\n+\t\t\t\t  remote_head_points_at->peer_ref->name,\n+\t\t\t\t  msg, &err)) {\n+\t\t\terror(\"%s\", err.buf);\n+\t\t\tstrbuf_release(&err);\n+\t\t}\n \t}\n }\n \n@@ -599,7 +603,10 @@ static void update_head(const struct ref *our, const struct ref *remote,\n \tconst char *head;\n \tif (our && skip_prefix(our->name, \"refs/heads/\", &head)) {\n \t\t/* Local default branch link */\n-\t\tcreate_symref(\"HEAD\", our->name, NULL);\n+\t\tif (create_symref(\"HEAD\", our->name, NULL, &err)) {\n+\t\t\terror(\"%s\", err.buf);\n+\t\t\tstrbuf_release(&err);\n+\t\t}\n \t\tif (!option_bare) {\n \t\t\tupdate_ref(msg, \"HEAD\", our->old_sha1, NULL, 0, &err);\n \t\t\tinstall_branch_config(0, head, option_origin, our->name);\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 587a505..d6cdee8 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -7,6 +7,7 @@\n #include \"builtin.h\"\n #include \"exec_cmd.h\"\n #include \"parse-options.h\"\n+#include \"refs.h\"\n \n #ifndef DEFAULT_GIT_TEMPLATE_DIR\n #define DEFAULT_GIT_TEMPLATE_DIR \"/usr/share/git-core/templates\"\n@@ -187,6 +188,7 @@ static int create_default_files(const char *template_path)\n \tchar junk[2];\n \tint reinit;\n \tint filemode;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tif (len > sizeof(path)-50)\n \t\tdie(_(\"insane git directory %s\"), git_dir);\n@@ -236,8 +238,10 @@ static int create_default_files(const char *template_path)\n \tstrcpy(path + len, \"HEAD\");\n \treinit = (!access(path, R_OK)\n \t\t  || readlink(path, junk, sizeof(junk)-1) != -1);\n-\tif (!reinit) {\n-\t\tif (create_symref(\"HEAD\", \"refs/heads/master\", NULL) < 0)\n+\tif (!reinit &&\n+\t    create_symref(\"HEAD\", \"refs/heads/master\", NULL, &err)) {\n+\t\t\terror(\"%s\", err.buf);\n+\t\t\tstrbuf_release(&err);\n \t\t\texit(1);\n \t}\n \ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex b9fec39..f6d4696 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -821,9 +821,10 @@ static int merge(int argc, const char **argv, const char *prefix)\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)\",\n-\t\t\t    default_notes_ref());\n+\t\tif (create_symref(\"NOTES_MERGE_REF\", default_notes_ref(),\n+\t\t\t\t  NULL, &err))\n+\t\t\tdie(\"Failed to store link to current notes ref (%s). \"\n+\t\t\t    \"%s\", default_notes_ref(), err.buf);\n \t\tprintf(\"Automatic notes merge failed. Fix conflicts in %s and \"\n \t\t       \"commit the result with 'git notes merge --commit', or \"\n \t\t       \"abort the merge with 'git notes merge --abort'.\\n\",\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 42702d7..d9632df 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -147,6 +147,7 @@ static int add(int argc, const char **argv)\n \tconst char *master = NULL;\n \tstruct remote *remote;\n \tstruct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \tconst char *name, *url;\n \tint i;\n \n@@ -230,8 +231,12 @@ static int add(int argc, const char **argv)\n \t\tstrbuf_reset(&buf2);\n \t\tstrbuf_addf(&buf2, \"refs/remotes/%s/%s\", name, master);\n \n-\t\tif (create_symref(buf.buf, buf2.buf, \"remote add\"))\n-\t\t\treturn error(_(\"Could not setup master '%s'\"), master);\n+\t\tif (create_symref(buf.buf, buf2.buf, \"remote add\", &err)) {\n+\t\t\terror(_(\"Could not setup master '%s'. %s\"),\n+\t\t\t      master, err.buf);\n+\t\t\tstrbuf_release(&err);\n+\t\t\treturn -1;\n+\t\t}\n \t}\n \n \tstrbuf_release(&buf);\n@@ -617,8 +622,8 @@ static int mv(int argc, const char **argv)\n \t\tOPT_END()\n \t};\n \tstruct remote *oldremote, *newremote;\n-\tstruct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT,\n-\t\told_remote_context = STRBUF_INIT;\n+\tstruct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT;\n+\tstruct strbuf old_remote_context = STRBUF_INIT, err = STRBUF_INIT;\n \tstruct string_list remote_branches = STRING_LIST_INIT_NODUP;\n \tstruct rename_info rename;\n \tint i, refspec_updated = 0;\n@@ -742,8 +747,8 @@ static int mv(int argc, const char **argv)\n \t\tstrbuf_reset(&buf3);\n \t\tstrbuf_addf(&buf3, \"remote: renamed %s to %s\",\n \t\t\t\titem->string, buf.buf);\n-\t\tif (create_symref(buf.buf, buf2.buf, buf3.buf))\n-\t\t\tdie(_(\"creating '%s' failed\"), buf.buf);\n+\t\tif (create_symref(buf.buf, buf2.buf, buf3.buf, &err))\n+\t\t\tdie(_(\"creating '%s' failed. %s\"), buf.buf, err.buf);\n \t}\n \treturn 0;\n }\n@@ -1260,6 +1265,7 @@ static int set_head(int argc, const char **argv)\n {\n \tint i, opt_a = 0, opt_d = 0, result = 0;\n \tstruct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \tchar *head_name = NULL;\n \n \tstruct option options[] = {\n@@ -1302,8 +1308,12 @@ static int set_head(int argc, const char **argv)\n \t\t/* make sure it's valid */\n \t\tif (!ref_exists(buf2.buf))\n \t\t\tresult |= error(_(\"Not a valid ref: %s\"), buf2.buf);\n-\t\telse if (create_symref(buf.buf, buf2.buf, \"remote set-head\"))\n-\t\t\tresult |= error(_(\"Could not setup %s\"), buf.buf);\n+\t\telse if (create_symref(buf.buf, buf2.buf, \"remote set-head\",\n+\t\t\t\t       &err)) {\n+\t\t\terror(_(\"Could not setup %s. %s\"), buf.buf, err.buf);\n+\t\t\tstrbuf_release(&err);\n+\t\t\tresult = -1;\n+\t\t}\n \t\tif (opt_a)\n \t\t\tprintf(\"%s/HEAD set to %s\\n\", argv[0], head_name);\n \t\tfree(head_name);\ndiff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c\nindex 29fb3f1..f9ca959 100644\n--- a/builtin/symbolic-ref.c\n+++ b/builtin/symbolic-ref.c\n@@ -35,6 +35,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)\n {\n \tint quiet = 0, delete = 0, shorten = 0, ret = 0;\n \tconst char *msg = NULL;\n+\tstruct strbuf err = STRBUF_INIT;\n \tstruct option options[] = {\n \t\tOPT__QUIET(&quiet,\n \t\t\tN_(\"suppress error message for non-symbolic (detached) refs\")),\n@@ -67,7 +68,10 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)\n \t\tif (!strcmp(argv[0], \"HEAD\") &&\n \t\t    !starts_with(argv[1], \"refs/\"))\n \t\t\tdie(\"Refusing to point HEAD outside of refs/\");\n-\t\tcreate_symref(argv[0], argv[1], msg);\n+\t\tif (create_symref(argv[0], argv[1], msg, &err)) {\n+\t\t\terror(\"%s\", err.buf);\n+\t\t\tstrbuf_release(&err);\n+\t\t}\n \t\tbreak;\n \tdefault:\n \t\tusage_with_options(git_symbolic_ref_usage, options);\ndiff --git a/cache.h b/cache.h\nindex 61e61af..3443da7 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1026,7 +1026,6 @@ extern int get_sha1_mb(const char *str, unsigned char *sha1);\n  */\n extern int refname_match(const char *abbrev_name, const char *full_name);\n \n-extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg);\n extern int validate_headref(const char *ref);\n \n extern int base_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);\ndiff --git a/refs.c b/refs.c\nindex fc9ace2..7a07856 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3104,20 +3104,22 @@ static int write_ref_sha1(struct ref_lock *lock,\n }\n \n int create_symref(const char *ref_target, const char *refs_heads_master,\n-\t\t  const char *logmsg)\n+\t\t  const char *logmsg, struct strbuf *err)\n {\n \tconst char *lockpath;\n \tchar ref[1000];\n \tint fd, len, written;\n \tchar *git_HEAD = git_pathdup(\"%s\", ref_target);\n \tunsigned char old_sha1[20], new_sha1[20];\n-\tstruct strbuf err = STRBUF_INIT;\n \n \tif (logmsg && read_ref(ref_target, old_sha1))\n \t\thashclr(old_sha1);\n \n-\tif (safe_create_leading_directories(git_HEAD) < 0)\n-\t\treturn error(\"unable to create directory for %s\", git_HEAD);\n+\tif (safe_create_leading_directories(git_HEAD) < 0) {\n+\t\tstrbuf_addf(err, \"unable to create directory for %s.\",\n+\t\t\t    git_HEAD);\n+\t\treturn -1;\n+\t}\n \n #ifndef NO_SYMLINK_HEAD\n \tif (prefer_symlink_refs) {\n@@ -3130,26 +3132,29 @@ int create_symref(const char *ref_target, const char *refs_heads_master,\n \n \tlen = snprintf(ref, sizeof(ref), \"ref: %s\\n\", refs_heads_master);\n \tif (sizeof(ref) <= len) {\n-\t\terror(\"refname too long: %s\", refs_heads_master);\n+\t\tstrbuf_addf(err, \"refname too long: %s\", refs_heads_master);\n \t\tgoto error_free_return;\n \t}\n \tlockpath = mkpath(\"%s.lock\", git_HEAD);\n \tfd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666);\n \tif (fd < 0) {\n-\t\terror(\"Unable to open %s for writing\", lockpath);\n+\t\tstrbuf_addf(err, \"Unable to open %s for writing. %s\", lockpath,\n+\t\t\t    strerror(errno));\n \t\tgoto error_free_return;\n \t}\n \twritten = write_in_full(fd, ref, len);\n \tif (close(fd) != 0 || written != len) {\n-\t\terror(\"Unable to write to %s\", lockpath);\n+\t\tstrbuf_addf(err, \"Unable to write to %s. %s\", lockpath,\n+\t\t\t    strerror(errno));\n \t\tgoto error_unlink_return;\n \t}\n \tif (rename(lockpath, git_HEAD) < 0) {\n-\t\terror(\"Unable to create %s\", git_HEAD);\n+\t\tstrbuf_addf(err, \"Unable to create %s. %s\", git_HEAD,\n+\t\t\t    strerror(errno));\n \t\tgoto error_unlink_return;\n \t}\n \tif (adjust_shared_perm(git_HEAD)) {\n-\t\terror(\"Unable to fix permissions on %s\", lockpath);\n+\t\tstrbuf_addf(err, \"Unable to fix permissions on %s\", lockpath);\n \terror_unlink_return:\n \t\tunlink_or_warn(lockpath);\n \terror_free_return:\n@@ -3161,10 +3166,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,\n \tdone:\n #endif\n \tif (logmsg && !read_ref(refs_heads_master, new_sha1) &&\n-\t    log_ref_write(ref_target, old_sha1, new_sha1, logmsg, &err))\n-\t\terror(\"%s\", err.buf);\n+\t    log_ref_write(ref_target, old_sha1, new_sha1, logmsg, err)) {\n+\t\terror(\"%s\", err->buf);\n+\t\tstrbuf_release(err);\n+\t}\n \t\n-\tstrbuf_release(&err);\n \tfree(git_HEAD);\n \treturn 0;\n }\ndiff --git a/refs.h b/refs.h\nindex 930821e..6c99a57 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -116,6 +116,7 @@ static inline const char *has_glob_specials(const char *pattern)\n /* can be used to learn about broken ref and symref */\n extern int for_each_rawref(each_ref_fn, void *);\n \n+extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg, struct strbuf *err);\n extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);\n extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct string_list *refnames);\n \n-- \n2.1.0.rc2.206.gedb03e5\n"}]}