* [PATCH v2 1/7] receive-pack.c: add protocol support to negotiate atomic-push
2014-11-03 19:12 [PATCH v2 0/7] ref-transaction-send-pack Ronnie Sahlberg
@ 2014-11-03 19:12 ` Ronnie Sahlberg
2014-11-03 19:12 ` [PATCH v2 2/7] send-pack.c: add an --atomic-push command line argument Ronnie Sahlberg
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ronnie Sahlberg @ 2014-11-03 19:12 UTC (permalink / raw)
To: git; +Cc: Ronnie Sahlberg
This adds support to the protocol between send-pack and receive-pack to
* allow receive-pack to inform the client that it has atomic push capability
* allow send-pack to request atomic push back.
There is currently no setting in send-pack to actually request that atomic
pushes are to be used yet. This only adds protocol capability not ability
for the user to activate it.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
Documentation/technical/protocol-capabilities.txt | 12 ++++++++++--
builtin/receive-pack.c | 6 +++++-
send-pack.c | 6 ++++++
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index 0c92dee..26bc5b1 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -18,8 +18,9 @@ was sent. Server MUST NOT ignore capabilities that client requested
and server advertised. As a consequence of these rules, server MUST
NOT advertise capabilities it does not understand.
-The 'report-status', 'delete-refs', 'quiet', and 'push-cert' capabilities
-are sent and recognized by the receive-pack (push to server) process.
+The 'atomic-push', 'report-status', 'delete-refs', 'quiet', and 'push-cert'
+capabilities are sent and recognized by the receive-pack (push to server)
+process.
The 'ofs-delta' and 'side-band-64k' capabilities are sent and recognized
by both upload-pack and receive-pack protocols. The 'agent' capability
@@ -244,6 +245,13 @@ respond with the 'quiet' capability to suppress server-side progress
reporting if the local progress reporting is also being suppressed
(e.g., via `push -q`, or if stderr does not go to a tty).
+atomic-push
+-----------
+
+If the server sends the 'atomic-push' capability, it means it is
+capable of accepting atomic pushes. If the pushing client requests this
+capability, the server will update the refs in one single atomic transaction.
+
allow-tip-sha1-in-want
----------------------
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ccea9dc..65d9a7e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -40,6 +40,7 @@ static int transfer_unpack_limit = -1;
static int unpack_limit = 100;
static int report_status;
static int use_sideband;
+static int use_atomic_push;
static int quiet;
static int prefer_ofs_delta = 1;
static int auto_update_server_info;
@@ -171,7 +172,8 @@ static void show_ref(const char *path, const unsigned char *sha1)
struct strbuf cap = STRBUF_INIT;
strbuf_addstr(&cap,
- "report-status delete-refs side-band-64k quiet");
+ "report-status delete-refs side-band-64k quiet "
+ "atomic-push");
if (prefer_ofs_delta)
strbuf_addstr(&cap, " ofs-delta");
if (push_cert_nonce)
@@ -1178,6 +1180,8 @@ static struct command *read_head_info(struct sha1_array *shallow)
use_sideband = LARGE_PACKET_MAX;
if (parse_feature_request(feature_list, "quiet"))
quiet = 1;
+ if (parse_feature_request(feature_list, "atomic-push"))
+ use_atomic_push = 1;
}
if (!strcmp(line, "push-cert")) {
diff --git a/send-pack.c b/send-pack.c
index 949cb61..1ccc84c 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -294,6 +294,8 @@ int send_pack(struct send_pack_args *args,
int use_sideband = 0;
int quiet_supported = 0;
int agent_supported = 0;
+ int atomic_push_supported = 0;
+ int atomic_push = 0;
unsigned cmds_sent = 0;
int ret;
struct async demux;
@@ -314,6 +316,8 @@ int send_pack(struct send_pack_args *args,
agent_supported = 1;
if (server_supports("no-thin"))
args->use_thin_pack = 0;
+ if (server_supports("atomic-push"))
+ atomic_push_supported = 1;
if (args->push_cert) {
int len;
@@ -335,6 +339,8 @@ int send_pack(struct send_pack_args *args,
strbuf_addstr(&cap_buf, " side-band-64k");
if (quiet_supported && (args->quiet || !args->progress))
strbuf_addstr(&cap_buf, " quiet");
+ if (atomic_push)
+ strbuf_addstr(&cap_buf, " atomic-push");
if (agent_supported)
strbuf_addf(&cap_buf, " agent=%s", git_user_agent_sanitized());
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 2/7] send-pack.c: add an --atomic-push command line argument
2014-11-03 19:12 [PATCH v2 0/7] ref-transaction-send-pack Ronnie Sahlberg
2014-11-03 19:12 ` [PATCH v2 1/7] receive-pack.c: add protocol support to negotiate atomic-push Ronnie Sahlberg
@ 2014-11-03 19:12 ` Ronnie Sahlberg
2014-11-04 22:17 ` Stefan Beller
2014-11-03 19:12 ` [PATCH v2 3/7] receive-pack.c: use a single transaction when atomic-push is negotiated Ronnie Sahlberg
` (4 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Ronnie Sahlberg @ 2014-11-03 19:12 UTC (permalink / raw)
To: git; +Cc: Ronnie Sahlberg
This adds support to send-pack to to negotiate and use atomic pushes
iff the server supports it. Atomic pushes are activated by a new command
line flag --atomic-push.
In order to do this we also need to change the semantics for send_pack()
slightly. The existing send_pack() function actually don't sent all the
refs back to the server when multiple refs are involved, for example
when using --all. Several of the failure modes for pushes can already be
detected locally in the send_pack client based on the information from the
initial server side list of all the refs as generated by receive-pack.
Any such refs that we thus know would fail to push are thus pruned from
the list of refs we send to the server to update.
For atomic pushes, we have to deal thus with both failures that are detected
locally as well as failures that are reported back from the server. In order
to do so we treat all local failures as push failures too.
We introduce a new status code REF_STATUS_ATOMIC_PUSH_FAILED so we can
flag all refs that we would normally have tried to push to the server
but we did not due to local failures. This is to improve the error message
back to the end user to flag that "these refs failed to update since the
atomic push operation failed."
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
Documentation/git-send-pack.txt | 7 ++++++-
builtin/send-pack.c | 6 +++++-
remote.h | 3 ++-
send-pack.c | 39 ++++++++++++++++++++++++++++++++++-----
send-pack.h | 1 +
transport.c | 4 ++++
6 files changed, 52 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index 2a0de42..9296587 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another repository
SYNOPSIS
--------
[verse]
-'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]
+'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]
DESCRIPTION
-----------
@@ -62,6 +62,11 @@ be in a separate packet, and the list must end with a flush packet.
Send a "thin" pack, which records objects in deltified form based
on objects not included in the pack to reduce network traffic.
+--atomic-push::
+ Use an atomic transaction for updating the refs. If any of the refs
+ fails to update then the entire push will fail without changing any
+ refs.
+
<host>::
A remote host to house the repository. When this
part is specified, 'git-receive-pack' is invoked via
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index b564a77..93cb17c 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -13,7 +13,7 @@
#include "sha1-array.h"
static const char send_pack_usage[] =
-"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
+"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]\n"
" --all and explicit <ref> specification are mutually exclusive.";
static struct send_pack_args args;
@@ -170,6 +170,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
args.use_thin_pack = 1;
continue;
}
+ if (!strcmp(arg, "--atomic-push")) {
+ args.use_atomic_push = 1;
+ continue;
+ }
if (!strcmp(arg, "--stateless-rpc")) {
args.stateless_rpc = 1;
continue;
diff --git a/remote.h b/remote.h
index 8b62efd..f346524 100644
--- a/remote.h
+++ b/remote.h
@@ -115,7 +115,8 @@ struct ref {
REF_STATUS_REJECT_SHALLOW,
REF_STATUS_UPTODATE,
REF_STATUS_REMOTE_REJECT,
- REF_STATUS_EXPECTING_REPORT
+ REF_STATUS_EXPECTING_REPORT,
+ REF_STATUS_ATOMIC_PUSH_FAILED
} status;
char *remote_status;
struct ref *peer_ref; /* when renaming */
diff --git a/send-pack.c b/send-pack.c
index 1ccc84c..08602a8 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -190,7 +190,7 @@ static void advertise_shallow_grafts_buf(struct strbuf *sb)
for_each_commit_graft(advertise_shallow_grafts_cb, sb);
}
-static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args)
+static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args, int *atomic_push_failed)
{
if (!ref->peer_ref && !args->send_mirror)
return 0;
@@ -203,6 +203,12 @@ static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_a
case REF_STATUS_REJECT_NEEDS_FORCE:
case REF_STATUS_REJECT_STALE:
case REF_STATUS_REJECT_NODELETE:
+ if (atomic_push_failed) {
+ fprintf(stderr, "Atomic push failed for ref %s. "
+ "Status:%d\n", ref->name, ref->status);
+ *atomic_push_failed = 1;
+ }
+ /* fallthrough */
case REF_STATUS_UPTODATE:
return 0;
default:
@@ -250,7 +256,7 @@ static int generate_push_cert(struct strbuf *req_buf,
strbuf_addstr(&cert, "\n");
for (ref = remote_refs; ref; ref = ref->next) {
- if (!ref_update_to_be_sent(ref, args))
+ if (!ref_update_to_be_sent(ref, args, NULL))
continue;
update_seen = 1;
strbuf_addf(&cert, "%s %s %s\n",
@@ -297,7 +303,7 @@ int send_pack(struct send_pack_args *args,
int atomic_push_supported = 0;
int atomic_push = 0;
unsigned cmds_sent = 0;
- int ret;
+ int ret, atomic_push_failed = 0;
struct async demux;
const char *push_cert_nonce = NULL;
@@ -332,6 +338,11 @@ int send_pack(struct send_pack_args *args,
"Perhaps you should specify a branch such as 'master'.\n");
return 0;
}
+ if (args->use_atomic_push && !atomic_push_supported) {
+ fprintf(stderr, "Server does not support atomic-push.");
+ return -1;
+ }
+ atomic_push = atomic_push_supported && args->use_atomic_push;
if (status_report)
strbuf_addstr(&cap_buf, " report-status");
@@ -365,7 +376,8 @@ int send_pack(struct send_pack_args *args,
* the pack data.
*/
for (ref = remote_refs; ref; ref = ref->next) {
- if (!ref_update_to_be_sent(ref, args))
+ if (!ref_update_to_be_sent(ref, args,
+ args->use_atomic_push ? &atomic_push_failed : NULL))
continue;
if (!ref->deletion)
@@ -377,6 +389,23 @@ int send_pack(struct send_pack_args *args,
ref->status = REF_STATUS_EXPECTING_REPORT;
}
+ if (atomic_push_failed) {
+ for (ref = remote_refs; ref; ref = ref->next) {
+ if (!ref->peer_ref && !args->send_mirror)
+ continue;
+
+ switch (ref->status) {
+ case REF_STATUS_EXPECTING_REPORT:
+ ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
+ continue;
+ default:
+ ; /* do nothing */
+ }
+ }
+ fprintf(stderr, "Atomic push failed.");
+ return -1;
+ }
+
/*
* Finally, tell the other end!
*/
@@ -386,7 +415,7 @@ int send_pack(struct send_pack_args *args,
if (args->dry_run || args->push_cert)
continue;
- if (!ref_update_to_be_sent(ref, args))
+ if (!ref_update_to_be_sent(ref, args, NULL))
continue;
old_hex = sha1_to_hex(ref->old_sha1);
diff --git a/send-pack.h b/send-pack.h
index 5635457..7486e65 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -11,6 +11,7 @@ struct send_pack_args {
force_update:1,
use_thin_pack:1,
use_ofs_delta:1,
+ use_atomic_push:1,
dry_run:1,
push_cert:1,
stateless_rpc:1;
diff --git a/transport.c b/transport.c
index f70d62f..2111986 100644
--- a/transport.c
+++ b/transport.c
@@ -731,6 +731,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
ref->deletion ? NULL : ref->peer_ref,
"remote failed to report status", porcelain);
break;
+ case REF_STATUS_ATOMIC_PUSH_FAILED:
+ print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+ "atomic-push-failed", porcelain);
+ break;
case REF_STATUS_OK:
print_ok_ref_status(ref, porcelain);
break;
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/7] send-pack.c: add an --atomic-push command line argument
2014-11-03 19:12 ` [PATCH v2 2/7] send-pack.c: add an --atomic-push command line argument Ronnie Sahlberg
@ 2014-11-04 22:17 ` Stefan Beller
2014-11-04 22:20 ` Ronnie Sahlberg
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Beller @ 2014-11-04 22:17 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: git
On Mon, Nov 3, 2014 at 11:12 AM, Ronnie Sahlberg <sahlberg@google.com> wrote:
> This adds support to send-pack to to negotiate and use atomic pushes
/s/to to/to/
> iff the server supports it. Atomic pushes are activated by a new command
> line flag --atomic-push.
>
> In order to do this we also need to change the semantics for send_pack()
> slightly. The existing send_pack() function actually don't sent all the
> refs back to the server when multiple refs are involved, for example
> when using --all. Several of the failure modes for pushes can already be
> detected locally in the send_pack client based on the information from the
> initial server side list of all the refs as generated by receive-pack.
> Any such refs that we thus know would fail to push are thus pruned from
> the list of refs we send to the server to update.
>
> For atomic pushes, we have to deal thus with both failures that are detected
> locally as well as failures that are reported back from the server. In order
> to do so we treat all local failures as push failures too.
>
> We introduce a new status code REF_STATUS_ATOMIC_PUSH_FAILED so we can
> flag all refs that we would normally have tried to push to the server
> but we did not due to local failures. This is to improve the error message
> back to the end user to flag that "these refs failed to update since the
> atomic push operation failed."
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> ---
> Documentation/git-send-pack.txt | 7 ++++++-
> builtin/send-pack.c | 6 +++++-
> remote.h | 3 ++-
> send-pack.c | 39 ++++++++++++++++++++++++++++++++++-----
> send-pack.h | 1 +
> transport.c | 4 ++++
> 6 files changed, 52 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
> index 2a0de42..9296587 100644
> --- a/Documentation/git-send-pack.txt
> +++ b/Documentation/git-send-pack.txt
> @@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another repository
> SYNOPSIS
> --------
> [verse]
> -'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]
> +'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]
>
> DESCRIPTION
> -----------
> @@ -62,6 +62,11 @@ be in a separate packet, and the list must end with a flush packet.
> Send a "thin" pack, which records objects in deltified form based
> on objects not included in the pack to reduce network traffic.
>
> +--atomic-push::
> + Use an atomic transaction for updating the refs. If any of the refs
> + fails to update then the entire push will fail without changing any
> + refs.
> +
> <host>::
> A remote host to house the repository. When this
> part is specified, 'git-receive-pack' is invoked via
> diff --git a/builtin/send-pack.c b/builtin/send-pack.c
> index b564a77..93cb17c 100644
> --- a/builtin/send-pack.c
> +++ b/builtin/send-pack.c
> @@ -13,7 +13,7 @@
> #include "sha1-array.h"
>
> static const char send_pack_usage[] =
> -"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
> +"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]\n"
> " --all and explicit <ref> specification are mutually exclusive.";
>
> static struct send_pack_args args;
> @@ -170,6 +170,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
> args.use_thin_pack = 1;
> continue;
> }
> + if (!strcmp(arg, "--atomic-push")) {
> + args.use_atomic_push = 1;
> + continue;
> + }
> if (!strcmp(arg, "--stateless-rpc")) {
> args.stateless_rpc = 1;
> continue;
> diff --git a/remote.h b/remote.h
> index 8b62efd..f346524 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -115,7 +115,8 @@ struct ref {
> REF_STATUS_REJECT_SHALLOW,
> REF_STATUS_UPTODATE,
> REF_STATUS_REMOTE_REJECT,
> - REF_STATUS_EXPECTING_REPORT
> + REF_STATUS_EXPECTING_REPORT,
> + REF_STATUS_ATOMIC_PUSH_FAILED
> } status;
> char *remote_status;
> struct ref *peer_ref; /* when renaming */
> diff --git a/send-pack.c b/send-pack.c
> index 1ccc84c..08602a8 100644
> --- a/send-pack.c
> +++ b/send-pack.c
> @@ -190,7 +190,7 @@ static void advertise_shallow_grafts_buf(struct strbuf *sb)
> for_each_commit_graft(advertise_shallow_grafts_cb, sb);
> }
>
> -static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args)
> +static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args, int *atomic_push_failed)
> {
> if (!ref->peer_ref && !args->send_mirror)
> return 0;
> @@ -203,6 +203,12 @@ static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_a
> case REF_STATUS_REJECT_NEEDS_FORCE:
> case REF_STATUS_REJECT_STALE:
> case REF_STATUS_REJECT_NODELETE:
> + if (atomic_push_failed) {
> + fprintf(stderr, "Atomic push failed for ref %s. "
> + "Status:%d\n", ref->name, ref->status);
> + *atomic_push_failed = 1;
> + }
> + /* fallthrough */
> case REF_STATUS_UPTODATE:
> return 0;
> default:
> @@ -250,7 +256,7 @@ static int generate_push_cert(struct strbuf *req_buf,
> strbuf_addstr(&cert, "\n");
>
> for (ref = remote_refs; ref; ref = ref->next) {
> - if (!ref_update_to_be_sent(ref, args))
> + if (!ref_update_to_be_sent(ref, args, NULL))
> continue;
> update_seen = 1;
> strbuf_addf(&cert, "%s %s %s\n",
> @@ -297,7 +303,7 @@ int send_pack(struct send_pack_args *args,
> int atomic_push_supported = 0;
> int atomic_push = 0;
> unsigned cmds_sent = 0;
> - int ret;
> + int ret, atomic_push_failed = 0;
> struct async demux;
> const char *push_cert_nonce = NULL;
>
> @@ -332,6 +338,11 @@ int send_pack(struct send_pack_args *args,
> "Perhaps you should specify a branch such as 'master'.\n");
> return 0;
> }
> + if (args->use_atomic_push && !atomic_push_supported) {
> + fprintf(stderr, "Server does not support atomic-push.");
> + return -1;
> + }
> + atomic_push = atomic_push_supported && args->use_atomic_push;
>
> if (status_report)
> strbuf_addstr(&cap_buf, " report-status");
> @@ -365,7 +376,8 @@ int send_pack(struct send_pack_args *args,
> * the pack data.
> */
> for (ref = remote_refs; ref; ref = ref->next) {
> - if (!ref_update_to_be_sent(ref, args))
> + if (!ref_update_to_be_sent(ref, args,
> + args->use_atomic_push ? &atomic_push_failed : NULL))
> continue;
>
> if (!ref->deletion)
> @@ -377,6 +389,23 @@ int send_pack(struct send_pack_args *args,
> ref->status = REF_STATUS_EXPECTING_REPORT;
> }
>
> + if (atomic_push_failed) {
> + for (ref = remote_refs; ref; ref = ref->next) {
> + if (!ref->peer_ref && !args->send_mirror)
> + continue;
> +
> + switch (ref->status) {
> + case REF_STATUS_EXPECTING_REPORT:
> + ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
> + continue;
> + default:
> + ; /* do nothing */
> + }
> + }
> + fprintf(stderr, "Atomic push failed.");
> + return -1;
> + }
> +
> /*
> * Finally, tell the other end!
> */
> @@ -386,7 +415,7 @@ int send_pack(struct send_pack_args *args,
> if (args->dry_run || args->push_cert)
> continue;
>
> - if (!ref_update_to_be_sent(ref, args))
> + if (!ref_update_to_be_sent(ref, args, NULL))
> continue;
>
> old_hex = sha1_to_hex(ref->old_sha1);
> diff --git a/send-pack.h b/send-pack.h
> index 5635457..7486e65 100644
> --- a/send-pack.h
> +++ b/send-pack.h
> @@ -11,6 +11,7 @@ struct send_pack_args {
> force_update:1,
> use_thin_pack:1,
> use_ofs_delta:1,
> + use_atomic_push:1,
> dry_run:1,
> push_cert:1,
> stateless_rpc:1;
> diff --git a/transport.c b/transport.c
> index f70d62f..2111986 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -731,6 +731,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
> ref->deletion ? NULL : ref->peer_ref,
> "remote failed to report status", porcelain);
> break;
> + case REF_STATUS_ATOMIC_PUSH_FAILED:
> + print_ref_status('!', "[rejected]", ref, ref->peer_ref,
> + "atomic-push-failed", porcelain);
> + break;
> case REF_STATUS_OK:
> print_ok_ref_status(ref, porcelain);
> break;
> --
> 2.1.0.rc2.206.gedb03e5
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/7] send-pack.c: add an --atomic-push command line argument
2014-11-04 22:17 ` Stefan Beller
@ 2014-11-04 22:20 ` Ronnie Sahlberg
0 siblings, 0 replies; 10+ messages in thread
From: Ronnie Sahlberg @ 2014-11-04 22:20 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
Fixed. Thanks
On Tue, Nov 4, 2014 at 2:17 PM, Stefan Beller <sbeller@google.com> wrote:
> On Mon, Nov 3, 2014 at 11:12 AM, Ronnie Sahlberg <sahlberg@google.com> wrote:
>> This adds support to send-pack to to negotiate and use atomic pushes
>
> /s/to to/to/
>
>> iff the server supports it. Atomic pushes are activated by a new command
>> line flag --atomic-push.
>>
>> In order to do this we also need to change the semantics for send_pack()
>> slightly. The existing send_pack() function actually don't sent all the
>> refs back to the server when multiple refs are involved, for example
>> when using --all. Several of the failure modes for pushes can already be
>> detected locally in the send_pack client based on the information from the
>> initial server side list of all the refs as generated by receive-pack.
>> Any such refs that we thus know would fail to push are thus pruned from
>> the list of refs we send to the server to update.
>>
>> For atomic pushes, we have to deal thus with both failures that are detected
>> locally as well as failures that are reported back from the server. In order
>> to do so we treat all local failures as push failures too.
>>
>> We introduce a new status code REF_STATUS_ATOMIC_PUSH_FAILED so we can
>> flag all refs that we would normally have tried to push to the server
>> but we did not due to local failures. This is to improve the error message
>> back to the end user to flag that "these refs failed to update since the
>> atomic push operation failed."
>>
>> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
>> ---
>> Documentation/git-send-pack.txt | 7 ++++++-
>> builtin/send-pack.c | 6 +++++-
>> remote.h | 3 ++-
>> send-pack.c | 39 ++++++++++++++++++++++++++++++++++-----
>> send-pack.h | 1 +
>> transport.c | 4 ++++
>> 6 files changed, 52 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
>> index 2a0de42..9296587 100644
>> --- a/Documentation/git-send-pack.txt
>> +++ b/Documentation/git-send-pack.txt
>> @@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another repository
>> SYNOPSIS
>> --------
>> [verse]
>> -'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]
>> +'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]
>>
>> DESCRIPTION
>> -----------
>> @@ -62,6 +62,11 @@ be in a separate packet, and the list must end with a flush packet.
>> Send a "thin" pack, which records objects in deltified form based
>> on objects not included in the pack to reduce network traffic.
>>
>> +--atomic-push::
>> + Use an atomic transaction for updating the refs. If any of the refs
>> + fails to update then the entire push will fail without changing any
>> + refs.
>> +
>> <host>::
>> A remote host to house the repository. When this
>> part is specified, 'git-receive-pack' is invoked via
>> diff --git a/builtin/send-pack.c b/builtin/send-pack.c
>> index b564a77..93cb17c 100644
>> --- a/builtin/send-pack.c
>> +++ b/builtin/send-pack.c
>> @@ -13,7 +13,7 @@
>> #include "sha1-array.h"
>>
>> static const char send_pack_usage[] =
>> -"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
>> +"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]\n"
>> " --all and explicit <ref> specification are mutually exclusive.";
>>
>> static struct send_pack_args args;
>> @@ -170,6 +170,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
>> args.use_thin_pack = 1;
>> continue;
>> }
>> + if (!strcmp(arg, "--atomic-push")) {
>> + args.use_atomic_push = 1;
>> + continue;
>> + }
>> if (!strcmp(arg, "--stateless-rpc")) {
>> args.stateless_rpc = 1;
>> continue;
>> diff --git a/remote.h b/remote.h
>> index 8b62efd..f346524 100644
>> --- a/remote.h
>> +++ b/remote.h
>> @@ -115,7 +115,8 @@ struct ref {
>> REF_STATUS_REJECT_SHALLOW,
>> REF_STATUS_UPTODATE,
>> REF_STATUS_REMOTE_REJECT,
>> - REF_STATUS_EXPECTING_REPORT
>> + REF_STATUS_EXPECTING_REPORT,
>> + REF_STATUS_ATOMIC_PUSH_FAILED
>> } status;
>> char *remote_status;
>> struct ref *peer_ref; /* when renaming */
>> diff --git a/send-pack.c b/send-pack.c
>> index 1ccc84c..08602a8 100644
>> --- a/send-pack.c
>> +++ b/send-pack.c
>> @@ -190,7 +190,7 @@ static void advertise_shallow_grafts_buf(struct strbuf *sb)
>> for_each_commit_graft(advertise_shallow_grafts_cb, sb);
>> }
>>
>> -static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args)
>> +static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args, int *atomic_push_failed)
>> {
>> if (!ref->peer_ref && !args->send_mirror)
>> return 0;
>> @@ -203,6 +203,12 @@ static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_a
>> case REF_STATUS_REJECT_NEEDS_FORCE:
>> case REF_STATUS_REJECT_STALE:
>> case REF_STATUS_REJECT_NODELETE:
>> + if (atomic_push_failed) {
>> + fprintf(stderr, "Atomic push failed for ref %s. "
>> + "Status:%d\n", ref->name, ref->status);
>> + *atomic_push_failed = 1;
>> + }
>> + /* fallthrough */
>> case REF_STATUS_UPTODATE:
>> return 0;
>> default:
>> @@ -250,7 +256,7 @@ static int generate_push_cert(struct strbuf *req_buf,
>> strbuf_addstr(&cert, "\n");
>>
>> for (ref = remote_refs; ref; ref = ref->next) {
>> - if (!ref_update_to_be_sent(ref, args))
>> + if (!ref_update_to_be_sent(ref, args, NULL))
>> continue;
>> update_seen = 1;
>> strbuf_addf(&cert, "%s %s %s\n",
>> @@ -297,7 +303,7 @@ int send_pack(struct send_pack_args *args,
>> int atomic_push_supported = 0;
>> int atomic_push = 0;
>> unsigned cmds_sent = 0;
>> - int ret;
>> + int ret, atomic_push_failed = 0;
>> struct async demux;
>> const char *push_cert_nonce = NULL;
>>
>> @@ -332,6 +338,11 @@ int send_pack(struct send_pack_args *args,
>> "Perhaps you should specify a branch such as 'master'.\n");
>> return 0;
>> }
>> + if (args->use_atomic_push && !atomic_push_supported) {
>> + fprintf(stderr, "Server does not support atomic-push.");
>> + return -1;
>> + }
>> + atomic_push = atomic_push_supported && args->use_atomic_push;
>>
>> if (status_report)
>> strbuf_addstr(&cap_buf, " report-status");
>> @@ -365,7 +376,8 @@ int send_pack(struct send_pack_args *args,
>> * the pack data.
>> */
>> for (ref = remote_refs; ref; ref = ref->next) {
>> - if (!ref_update_to_be_sent(ref, args))
>> + if (!ref_update_to_be_sent(ref, args,
>> + args->use_atomic_push ? &atomic_push_failed : NULL))
>> continue;
>>
>> if (!ref->deletion)
>> @@ -377,6 +389,23 @@ int send_pack(struct send_pack_args *args,
>> ref->status = REF_STATUS_EXPECTING_REPORT;
>> }
>>
>> + if (atomic_push_failed) {
>> + for (ref = remote_refs; ref; ref = ref->next) {
>> + if (!ref->peer_ref && !args->send_mirror)
>> + continue;
>> +
>> + switch (ref->status) {
>> + case REF_STATUS_EXPECTING_REPORT:
>> + ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
>> + continue;
>> + default:
>> + ; /* do nothing */
>> + }
>> + }
>> + fprintf(stderr, "Atomic push failed.");
>> + return -1;
>> + }
>> +
>> /*
>> * Finally, tell the other end!
>> */
>> @@ -386,7 +415,7 @@ int send_pack(struct send_pack_args *args,
>> if (args->dry_run || args->push_cert)
>> continue;
>>
>> - if (!ref_update_to_be_sent(ref, args))
>> + if (!ref_update_to_be_sent(ref, args, NULL))
>> continue;
>>
>> old_hex = sha1_to_hex(ref->old_sha1);
>> diff --git a/send-pack.h b/send-pack.h
>> index 5635457..7486e65 100644
>> --- a/send-pack.h
>> +++ b/send-pack.h
>> @@ -11,6 +11,7 @@ struct send_pack_args {
>> force_update:1,
>> use_thin_pack:1,
>> use_ofs_delta:1,
>> + use_atomic_push:1,
>> dry_run:1,
>> push_cert:1,
>> stateless_rpc:1;
>> diff --git a/transport.c b/transport.c
>> index f70d62f..2111986 100644
>> --- a/transport.c
>> +++ b/transport.c
>> @@ -731,6 +731,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
>> ref->deletion ? NULL : ref->peer_ref,
>> "remote failed to report status", porcelain);
>> break;
>> + case REF_STATUS_ATOMIC_PUSH_FAILED:
>> + print_ref_status('!', "[rejected]", ref, ref->peer_ref,
>> + "atomic-push-failed", porcelain);
>> + break;
>> case REF_STATUS_OK:
>> print_ok_ref_status(ref, porcelain);
>> break;
>> --
>> 2.1.0.rc2.206.gedb03e5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/7] receive-pack.c: use a single transaction when atomic-push is negotiated
2014-11-03 19:12 [PATCH v2 0/7] ref-transaction-send-pack Ronnie Sahlberg
2014-11-03 19:12 ` [PATCH v2 1/7] receive-pack.c: add protocol support to negotiate atomic-push Ronnie Sahlberg
2014-11-03 19:12 ` [PATCH v2 2/7] send-pack.c: add an --atomic-push command line argument Ronnie Sahlberg
@ 2014-11-03 19:12 ` Ronnie Sahlberg
2014-11-03 19:12 ` [PATCH v2 4/7] push.c: add an --atomic-push argument Ronnie Sahlberg
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ronnie Sahlberg @ 2014-11-03 19:12 UTC (permalink / raw)
To: git; +Cc: Ronnie Sahlberg
Update receive-pack to use an atomic transaction iff the client negotiated
that it wanted atomic-push.
This leaves the default behavior to be the old non-atomic one ref at a
time update. This is to cause as little disruption as possible to existing
clients. It is unknown if there are client scripts that depend on the old
non-atomic behavior so we make it opt-in for now.
Later patch in this series also adds a configuration variable where you can
override the atomic push behavior on the receiving repo and force it
to use atomic updates always.
If it turns out over time that there are no client scripts that depend on the
old behavior we can change git to default to use atomic pushes and instead
offer an opt-out argument for people that do not want atomic pushes.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
builtin/receive-pack.c | 73 +++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 58 insertions(+), 15 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 65d9a7e..27b49dd 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -67,6 +67,8 @@ static const char *NONCE_SLOP = "SLOP";
static const char *nonce_status;
static long nonce_stamp_slop;
static unsigned long nonce_stamp_slop_limit;
+struct strbuf err = STRBUF_INIT;
+struct transaction *transaction;
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -832,33 +834,55 @@ static const char *update(struct command *cmd, struct shallow_info *si)
cmd->did_not_exist = 1;
}
}
- if (delete_ref(namespaced_name, old_sha1, 0)) {
- rp_error("failed to delete %s", name);
- return "failed to delete";
+ if (!use_atomic_push) {
+ if (delete_ref(namespaced_name, old_sha1, 0)) {
+ rp_error("failed to delete %s", name);
+ return "failed to delete";
+ }
+ } else {
+ if (transaction_delete_ref(transaction,
+ namespaced_name,
+ old_sha1,
+ 0, old_sha1 != NULL,
+ "push", &err)) {
+ rp_error("%s", err.buf);
+ strbuf_release(&err);
+ return "failed to delete";
+ }
}
return NULL; /* good */
}
else {
- struct strbuf err = STRBUF_INIT;
- struct transaction *transaction;
-
if (shallow_update && si->shallow_ref[cmd->index] &&
update_shallow_ref(cmd, si))
return "shallow error";
- transaction = transaction_begin(&err);
- if (!transaction ||
- transaction_update_ref(transaction, namespaced_name,
- new_sha1, old_sha1, 0, 1, "push",
- &err) ||
- transaction_commit(transaction, &err)) {
- transaction_free(transaction);
+ if (!use_atomic_push) {
+ transaction = transaction_begin(&err);
+ if (!transaction) {
+ rp_error("%s", err.buf);
+ strbuf_release(&err);
+ return "failed to start transaction";
+ }
+ }
+ if (transaction_update_ref(transaction,
+ namespaced_name,
+ new_sha1, old_sha1,
+ 0, 1, "push",
+ &err)) {
rp_error("%s", err.buf);
strbuf_release(&err);
return "failed to update ref";
}
-
- transaction_free(transaction);
+ if (!use_atomic_push) {
+ if (transaction_commit(transaction, &err)) {
+ transaction_free(transaction);
+ rp_error("%s", err.buf);
+ strbuf_release(&err);
+ return "failed to update ref";
+ }
+ transaction_free(transaction);
+ }
strbuf_release(&err);
return NULL; /* good */
}
@@ -1058,6 +1082,16 @@ static void execute_commands(struct command *commands,
return;
}
+ if (use_atomic_push) {
+ transaction = transaction_begin(&err);
+ if (!transaction) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ for (cmd = commands; cmd; cmd = cmd->next)
+ cmd->error_string = "transaction error";
+ return;
+ }
+ }
data.cmds = commands;
data.si = si;
if (check_everything_connected(iterate_receive_command_list, 0, &data))
@@ -1095,6 +1129,14 @@ static void execute_commands(struct command *commands,
}
}
+ if (use_atomic_push) {
+ if (transaction_commit(transaction, &err)) {
+ rp_error("%s", err.buf);
+ for (cmd = commands; cmd; cmd = cmd->next)
+ cmd->error_string = err.buf;
+ }
+ transaction_free(transaction);
+ }
if (shallow_update && !checked_connectivity)
error("BUG: run 'git fsck' for safety.\n"
"If there are errors, try to remove "
@@ -1542,5 +1584,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
sha1_array_clear(&shallow);
sha1_array_clear(&ref);
free((void *)push_cert_nonce);
+ strbuf_release(&err);
return 0;
}
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 4/7] push.c: add an --atomic-push argument
2014-11-03 19:12 [PATCH v2 0/7] ref-transaction-send-pack Ronnie Sahlberg
` (2 preceding siblings ...)
2014-11-03 19:12 ` [PATCH v2 3/7] receive-pack.c: use a single transaction when atomic-push is negotiated Ronnie Sahlberg
@ 2014-11-03 19:12 ` Ronnie Sahlberg
2014-11-03 19:12 ` [PATCH v2 5/7] t5543-atomic-push.sh: add basic tests for atomic pushes Ronnie Sahlberg
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ronnie Sahlberg @ 2014-11-03 19:12 UTC (permalink / raw)
To: git; +Cc: Ronnie Sahlberg
Add a command line argument to the git push command to request atomic
pushes.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
Documentation/git-push.txt | 7 ++++++-
builtin/push.c | 2 ++
transport.c | 1 +
transport.h | 1 +
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 21b3f29..04de8d8 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -9,7 +9,7 @@ git-push - Update remote refs along with associated objects
SYNOPSIS
--------
[verse]
-'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
+'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic-push] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
[--repo=<repository>] [-f | --force] [--prune] [-v | --verbose]
[-u | --set-upstream] [--signed]
[--force-with-lease[=<refname>[:<expect>]]]
@@ -136,6 +136,11 @@ already exists on the remote side.
logged. See linkgit:git-receive-pack[1] for the details
on the receiving end.
+--atomic-push::
+ Try using atomic push. If atomic push is negotiated with the server
+ then any push covering multiple refs will be atomic. Either all
+ refs are updated, or on error, no refs are updated.
+
--receive-pack=<git-receive-pack>::
--exec=<git-receive-pack>::
Path to the 'git-receive-pack' program on the remote
diff --git a/builtin/push.c b/builtin/push.c
index ae56f73..0b9f21a 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -507,6 +507,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
OPT_BIT(0, "follow-tags", &flags, N_("push missing but relevant tags"),
TRANSPORT_PUSH_FOLLOW_TAGS),
OPT_BIT(0, "signed", &flags, N_("GPG sign the push"), TRANSPORT_PUSH_CERT),
+ OPT_BIT(0, "atomic-push", &flags, N_("use atomic push, if available"),
+ TRANSPORT_ATOMIC_PUSH),
OPT_END()
};
diff --git a/transport.c b/transport.c
index 2111986..cd2b63a 100644
--- a/transport.c
+++ b/transport.c
@@ -833,6 +833,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);
args.push_cert = !!(flags & TRANSPORT_PUSH_CERT);
+ args.use_atomic_push = !!(flags & TRANSPORT_ATOMIC_PUSH);
args.url = transport->url;
ret = send_pack(&args, data->fd, data->conn, remote_refs,
diff --git a/transport.h b/transport.h
index 3e0091e..25fa1da 100644
--- a/transport.h
+++ b/transport.h
@@ -125,6 +125,7 @@ struct transport {
#define TRANSPORT_PUSH_NO_HOOK 512
#define TRANSPORT_PUSH_FOLLOW_TAGS 1024
#define TRANSPORT_PUSH_CERT 2048
+#define TRANSPORT_ATOMIC_PUSH 4096
#define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
#define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 5/7] t5543-atomic-push.sh: add basic tests for atomic pushes
2014-11-03 19:12 [PATCH v2 0/7] ref-transaction-send-pack Ronnie Sahlberg
` (3 preceding siblings ...)
2014-11-03 19:12 ` [PATCH v2 4/7] push.c: add an --atomic-push argument Ronnie Sahlberg
@ 2014-11-03 19:12 ` Ronnie Sahlberg
2014-11-03 19:12 ` [PATCH v2 6/7] refs.c: add an err argument to create_reflog Ronnie Sahlberg
2014-11-03 19:12 ` [PATCH v2 7/7] refs.c: add an err argument to create_symref Ronnie Sahlberg
6 siblings, 0 replies; 10+ messages in thread
From: Ronnie Sahlberg @ 2014-11-03 19:12 UTC (permalink / raw)
To: git; +Cc: Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
t/t5543-atomic-push.sh | 101 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)
create mode 100755 t/t5543-atomic-push.sh
diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
new file mode 100755
index 0000000..4903227
--- /dev/null
+++ b/t/t5543-atomic-push.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+
+test_description='pushing to a mirror repository'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+invert () {
+ if "$@"; then
+ return 1
+ else
+ return 0
+ fi
+}
+
+mk_repo_pair () {
+ rm -rf master mirror &&
+ mkdir mirror &&
+ (
+ cd mirror &&
+ git init &&
+ git config receive.denyCurrentBranch warn
+ ) &&
+ mkdir master &&
+ (
+ cd master &&
+ git init &&
+ git remote add $1 up ../mirror
+ )
+}
+
+
+test_expect_success 'atomic push works for a single branch' '
+
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git push --mirror up
+ echo two >foo && git add foo && git commit -m two &&
+ git push --atomic-push --mirror up
+ ) &&
+ master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+ mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+ test "$master_master" = "$mirror_master"
+
+'
+
+test_expect_success 'atomic push works for two branches' '
+
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git branch second &&
+ git push --mirror up
+ echo two >foo && git add foo && git commit -m two &&
+ git checkout second &&
+ echo three >foo && git add foo && git commit -m three &&
+ git checkout master &&
+ git push --atomic-push --mirror up
+ ) &&
+ master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+ mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+ test "$master_master" = "$mirror_master"
+
+ master_second=$(cd master && git show-ref -s --verify refs/heads/second) &&
+ mirror_second=$(cd mirror && git show-ref -s --verify refs/heads/second) &&
+ test "$master_second" = "$mirror_second"
+'
+
+# set up two branches where master can be pushed but second can not
+# (non-fast-forward). Since second can not be pushed the whole operation
+# will fail and leave master untouched.
+test_expect_success 'atomic push fails if one branch fails' '
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git branch second &&
+ git checkout second &&
+ echo two >foo && git add foo && git commit -m two &&
+ echo three >foo && git add foo && git commit -m three &&
+ echo four >foo && git add foo && git commit -m four &&
+ git push --mirror up
+ git reset --hard HEAD~2 &&
+ git checkout master
+ echo five >foo && git add foo && git commit -m five &&
+ ! git push --atomic-push --all up
+ ) &&
+ master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+ mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+ test "$master_master" != "$mirror_master" &&
+
+ master_second=$(cd master && git show-ref -s --verify refs/heads/second) &&
+ mirror_second=$(cd mirror && git show-ref -s --verify refs/heads/second) &&
+ test "$master_second" != "$mirror_second"
+'
+
+test_done
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 6/7] refs.c: add an err argument to create_reflog
2014-11-03 19:12 [PATCH v2 0/7] ref-transaction-send-pack Ronnie Sahlberg
` (4 preceding siblings ...)
2014-11-03 19:12 ` [PATCH v2 5/7] t5543-atomic-push.sh: add basic tests for atomic pushes Ronnie Sahlberg
@ 2014-11-03 19:12 ` Ronnie Sahlberg
2014-11-03 19:12 ` [PATCH v2 7/7] refs.c: add an err argument to create_symref Ronnie Sahlberg
6 siblings, 0 replies; 10+ messages in thread
From: Ronnie Sahlberg @ 2014-11-03 19:12 UTC (permalink / raw)
To: git; +Cc: Ronnie Sahlberg
Add err argument to create_reflog that can explain the reason for a
failure. This then eliminates the need to manage errno through this
function since we can just add strerror(errno) to the err string when
meaningful. No callers relied on errno from this function for anything
else than the error message.
log_ref_write is a private function that calls create_reflog. Update
this function to also take an err argument and pass it back to the caller.
This again eliminates the need to manage errno in this function.
Update the private function write_sha1_update_reflog to also take an
err argument.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
builtin/checkout.c | 8 +++---
refs.c | 71 +++++++++++++++++++++++++++---------------------------
refs.h | 4 +--
3 files changed, 42 insertions(+), 41 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 60a68f7..d9cb9c3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -590,10 +590,12 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
if (opts->new_orphan_branch) {
if (opts->new_branch_log && !log_all_ref_updates) {
char *ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
+ struct strbuf err = STRBUF_INIT;
- if (create_reflog(ref_name)) {
- fprintf(stderr, _("Can not do reflog for '%s'\n"),
- opts->new_orphan_branch);
+ if (create_reflog(ref_name, &err)) {
+ fprintf(stderr, _("Can not do reflog for '%s'. %s\n"),
+ opts->new_orphan_branch, err.buf);
+ strbuf_release(&err);
return;
}
}
diff --git a/refs.c b/refs.c
index c98d594..04ed571 100644
--- a/refs.c
+++ b/refs.c
@@ -2900,8 +2900,7 @@ static int copy_msg(char *buf, const char *msg)
return cp - buf;
}
-/* This function must set a meaningful errno on failure */
-int create_reflog(const char *refname)
+int create_reflog(const char *refname, struct strbuf *err)
{
int logfd, oflags = O_APPEND | O_WRONLY;
char logfile[PATH_MAX];
@@ -2912,9 +2911,8 @@ int create_reflog(const char *refname)
starts_with(refname, "refs/notes/") ||
!strcmp(refname, "HEAD")) {
if (safe_create_leading_directories(logfile) < 0) {
- int save_errno = errno;
- error("unable to create directory for %s", logfile);
- errno = save_errno;
+ strbuf_addf(err, "unable to create directory for %s. "
+ "%s", logfile, strerror(errno));
return -1;
}
oflags |= O_CREAT;
@@ -2927,20 +2925,16 @@ int create_reflog(const char *refname)
if ((oflags & O_CREAT) && errno == EISDIR) {
if (remove_empty_directories(logfile)) {
- int save_errno = errno;
- error("There are still logs under '%s'",
- logfile);
- errno = save_errno;
+ strbuf_addf(err, "There are still logs under "
+ "'%s'", logfile);
return -1;
}
logfd = open(logfile, oflags, 0666);
}
if (logfd < 0) {
- int save_errno = errno;
- error("Unable to append to %s: %s", logfile,
- strerror(errno));
- errno = save_errno;
+ strbuf_addf(err, "Unable to append to %s: %s",
+ logfile, strerror(errno));
return -1;
}
}
@@ -2977,7 +2971,8 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
}
static int log_ref_write(const char *refname, const unsigned char *old_sha1,
- const unsigned char *new_sha1, const char *msg)
+ const unsigned char *new_sha1, const char *msg,
+ struct strbuf *err)
{
int logfd, result = 0, oflags = O_APPEND | O_WRONLY;
char log_file[PATH_MAX];
@@ -2986,7 +2981,7 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
log_all_ref_updates = !is_bare_repository();
if (log_all_ref_updates && !reflog_exists(refname))
- result = create_reflog(refname);
+ result = create_reflog(refname, err);
if (result)
return result;
@@ -2999,16 +2994,14 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
result = log_ref_write_fd(logfd, old_sha1, new_sha1,
git_committer_info(0), msg);
if (result) {
- int save_errno = errno;
close(logfd);
- error("Unable to append to %s", log_file);
- errno = save_errno;
+ strbuf_addf(err, "Unable to append to %s. %s", log_file,
+ strerror(errno));
return -1;
}
if (close(logfd)) {
- int save_errno = errno;
- error("Unable to append to %s", log_file);
- errno = save_errno;
+ strbuf_addf(err, "Unable to append to %s. %s", log_file,
+ strerror(errno));
return -1;
}
return 0;
@@ -3020,12 +3013,12 @@ int is_branch(const char *refname)
}
static int write_sha1_update_reflog(struct ref_lock *lock,
- const unsigned char *sha1, const char *logmsg)
+ const unsigned char *sha1,
+ const char *logmsg, struct strbuf *err)
{
- if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
+ if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg, err) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
- log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
- unlock_ref(lock);
+ log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg, err) < 0)) {
return -1;
}
if (strcmp(lock->orig_ref_name, "HEAD") != 0) {
@@ -3047,8 +3040,11 @@ static int write_sha1_update_reflog(struct ref_lock *lock,
head_ref = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
head_sha1, &head_flag);
if (head_ref && (head_flag & REF_ISSYMREF) &&
- !strcmp(head_ref, lock->ref_name))
- log_ref_write("HEAD", lock->old_sha1, sha1, logmsg);
+ !strcmp(head_ref, lock->ref_name) &&
+ log_ref_write("HEAD", lock->old_sha1, sha1, logmsg, err)) {
+ error("%s", err->buf);
+ strbuf_release(err);
+ }
}
return 0;
}
@@ -3062,6 +3058,7 @@ static int write_ref_sha1(struct ref_lock *lock,
{
static char term = '\n';
struct object *o;
+ struct strbuf err = STRBUF_INIT;
if (!lock) {
errno = EINVAL;
@@ -3096,8 +3093,10 @@ static int write_ref_sha1(struct ref_lock *lock,
return -1;
}
clear_loose_ref_cache(&ref_cache);
- if (write_sha1_update_reflog(lock, sha1, logmsg)) {
+ if (write_sha1_update_reflog(lock, sha1, logmsg, &err)) {
unlock_ref(lock);
+ error("%s", err.buf);
+ strbuf_release(&err);
return -1;
}
if (commit_ref(lock)) {
@@ -3117,6 +3116,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
int fd, len, written;
char *git_HEAD = git_pathdup("%s", ref_target);
unsigned char old_sha1[20], new_sha1[20];
+ struct strbuf err = STRBUF_INIT;
if (logmsg && read_ref(ref_target, old_sha1))
hashclr(old_sha1);
@@ -3165,9 +3165,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
#ifndef NO_SYMLINK_HEAD
done:
#endif
- if (logmsg && !read_ref(refs_heads_master, new_sha1))
- log_ref_write(ref_target, old_sha1, new_sha1, logmsg);
-
+ if (logmsg && !read_ref(refs_heads_master, new_sha1) &&
+ log_ref_write(ref_target, old_sha1, new_sha1, logmsg, &err))
+ error("%s", err.buf);
+
+ strbuf_release(&err);
free(git_HEAD);
return 0;
}
@@ -3938,10 +3940,7 @@ int transaction_commit(struct transaction *transaction,
goto cleanup;
}
if (write_sha1_update_reflog(update->lock, update->new_sha1,
- update->msg)) {
- if (err)
- strbuf_addf(err, "Failed to update log '%s'.",
- update->refname);
+ update->msg, err)) {
ret = -1;
goto cleanup;
}
@@ -3978,7 +3977,7 @@ int transaction_commit(struct transaction *transaction,
continue;
}
if (log_all_ref_updates && !reflog_exists(update->refname) &&
- create_reflog(update->refname)) {
+ create_reflog(update->refname, err)) {
ret = -1;
if (err)
strbuf_addf(err, "Failed to setup reflog for "
diff --git a/refs.h b/refs.h
index 7c2f6c0..78af3a6 100644
--- a/refs.h
+++ b/refs.h
@@ -170,8 +170,8 @@ extern int read_ref_at(const char *refname, unsigned int flags,
/** Check if a particular reflog exists */
extern int reflog_exists(const char *refname);
-/** Create reflog. Set errno to something meaningful on failure. */
-extern int create_reflog(const char *refname);
+/** Create reflog. Fill in err on failure. */
+extern int create_reflog(const char *refname, struct strbuf *err);
/** Delete a reflog */
extern int delete_reflog(const char *refname);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 7/7] refs.c: add an err argument to create_symref
2014-11-03 19:12 [PATCH v2 0/7] ref-transaction-send-pack Ronnie Sahlberg
` (5 preceding siblings ...)
2014-11-03 19:12 ` [PATCH v2 6/7] refs.c: add an err argument to create_reflog Ronnie Sahlberg
@ 2014-11-03 19:12 ` Ronnie Sahlberg
6 siblings, 0 replies; 10+ messages in thread
From: Ronnie Sahlberg @ 2014-11-03 19:12 UTC (permalink / raw)
To: git; +Cc: Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
builtin/branch.c | 7 +++++--
builtin/checkout.c | 13 ++++++++++---
builtin/clone.c | 15 +++++++++++----
builtin/init-db.c | 8 ++++++--
builtin/notes.c | 7 ++++---
builtin/remote.c | 26 ++++++++++++++++++--------
builtin/symbolic-ref.c | 6 +++++-
cache.h | 1 -
refs.c | 30 ++++++++++++++++++------------
refs.h | 1 +
10 files changed, 78 insertions(+), 36 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 04f57d4..ab6d9f4 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -698,6 +698,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
{
struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
+ struct strbuf err = STRBUF_INIT;
int recovery = 0;
int clobber_head_ok;
@@ -734,8 +735,10 @@ static void rename_branch(const char *oldname, const char *newname, int force)
warning(_("Renamed a misnamed branch '%s' away"), oldref.buf + 11);
/* no need to pass logmsg here as HEAD didn't really move */
- if (!strcmp(oldname, head) && create_symref("HEAD", newref.buf, NULL))
- die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+ if (!strcmp(oldname, head) &&
+ create_symref("HEAD", newref.buf, NULL, &err))
+ die(_("Branch renamed to %s, but HEAD is not updated!. %s"),
+ newname, err.buf);
strbuf_addf(&oldsection, "branch.%s", oldref.buf + 11);
strbuf_release(&oldref);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index d9cb9c3..1efe353 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -634,7 +634,10 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
describe_detached_head(_("HEAD is now at"), new->commit);
}
} else if (new->path) { /* Switch branches. */
- create_symref("HEAD", new->path, msg.buf);
+ if (create_symref("HEAD", new->path, msg.buf, &err)) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ }
if (!opts->quiet) {
if (old->path && !strcmp(new->path, old->path)) {
if (opts->new_branch_force)
@@ -1020,12 +1023,16 @@ static int parse_branchname_arg(int argc, const char **argv,
static int switch_unborn_to_new_branch(const struct checkout_opts *opts)
{
int status;
- struct strbuf branch_ref = STRBUF_INIT;
+ struct strbuf branch_ref = STRBUF_INIT, err = STRBUF_INIT;
if (!opts->new_branch)
die(_("You are on a branch yet to be born"));
strbuf_addf(&branch_ref, "refs/heads/%s", opts->new_branch);
- status = create_symref("HEAD", branch_ref.buf, "checkout -b");
+ status = create_symref("HEAD", branch_ref.buf, "checkout -b", &err);
+ if (status) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ }
strbuf_release(&branch_ref);
if (!opts->quiet)
fprintf(stderr, _("Switched to a new branch '%s'\n"),
diff --git a/builtin/clone.c b/builtin/clone.c
index 5577b5b..17b6ae8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -565,6 +565,7 @@ static void update_remote_refs(const struct ref *refs,
int check_connectivity)
{
const struct ref *rm = mapped_refs;
+ struct strbuf err = STRBUF_INIT;
if (check_connectivity) {
if (transport->progress)
@@ -586,9 +587,12 @@ static void update_remote_refs(const struct ref *refs,
struct strbuf head_ref = STRBUF_INIT;
strbuf_addstr(&head_ref, branch_top);
strbuf_addstr(&head_ref, "HEAD");
- create_symref(head_ref.buf,
- remote_head_points_at->peer_ref->name,
- msg);
+ if (create_symref(head_ref.buf,
+ remote_head_points_at->peer_ref->name,
+ msg, &err)) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ }
}
}
@@ -599,7 +603,10 @@ static void update_head(const struct ref *our, const struct ref *remote,
const char *head;
if (our && skip_prefix(our->name, "refs/heads/", &head)) {
/* Local default branch link */
- create_symref("HEAD", our->name, NULL);
+ if (create_symref("HEAD", our->name, NULL, &err)) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ }
if (!option_bare) {
update_ref(msg, "HEAD", our->old_sha1, NULL, 0, &err);
install_branch_config(0, head, option_origin, our->name);
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 587a505..d6cdee8 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -7,6 +7,7 @@
#include "builtin.h"
#include "exec_cmd.h"
#include "parse-options.h"
+#include "refs.h"
#ifndef DEFAULT_GIT_TEMPLATE_DIR
#define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
@@ -187,6 +188,7 @@ static int create_default_files(const char *template_path)
char junk[2];
int reinit;
int filemode;
+ struct strbuf err = STRBUF_INIT;
if (len > sizeof(path)-50)
die(_("insane git directory %s"), git_dir);
@@ -236,8 +238,10 @@ static int create_default_files(const char *template_path)
strcpy(path + len, "HEAD");
reinit = (!access(path, R_OK)
|| readlink(path, junk, sizeof(junk)-1) != -1);
- if (!reinit) {
- if (create_symref("HEAD", "refs/heads/master", NULL) < 0)
+ if (!reinit &&
+ create_symref("HEAD", "refs/heads/master", NULL, &err)) {
+ error("%s", err.buf);
+ strbuf_release(&err);
exit(1);
}
diff --git a/builtin/notes.c b/builtin/notes.c
index b9fec39..f6d4696 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -821,9 +821,10 @@ static int merge(int argc, const char **argv, const char *prefix)
0, &err))
die("%s", err.buf);
/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
- if (create_symref("NOTES_MERGE_REF", default_notes_ref(), NULL))
- die("Failed to store link to current notes ref (%s)",
- default_notes_ref());
+ if (create_symref("NOTES_MERGE_REF", default_notes_ref(),
+ NULL, &err))
+ die("Failed to store link to current notes ref (%s). "
+ "%s", default_notes_ref(), err.buf);
printf("Automatic notes merge failed. Fix conflicts in %s and "
"commit the result with 'git notes merge --commit', or "
"abort the merge with 'git notes merge --abort'.\n",
diff --git a/builtin/remote.c b/builtin/remote.c
index 42702d7..d9632df 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -147,6 +147,7 @@ static int add(int argc, const char **argv)
const char *master = NULL;
struct remote *remote;
struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
+ struct strbuf err = STRBUF_INIT;
const char *name, *url;
int i;
@@ -230,8 +231,12 @@ static int add(int argc, const char **argv)
strbuf_reset(&buf2);
strbuf_addf(&buf2, "refs/remotes/%s/%s", name, master);
- if (create_symref(buf.buf, buf2.buf, "remote add"))
- return error(_("Could not setup master '%s'"), master);
+ if (create_symref(buf.buf, buf2.buf, "remote add", &err)) {
+ error(_("Could not setup master '%s'. %s"),
+ master, err.buf);
+ strbuf_release(&err);
+ return -1;
+ }
}
strbuf_release(&buf);
@@ -617,8 +622,8 @@ static int mv(int argc, const char **argv)
OPT_END()
};
struct remote *oldremote, *newremote;
- struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT,
- old_remote_context = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT;
+ struct strbuf old_remote_context = STRBUF_INIT, err = STRBUF_INIT;
struct string_list remote_branches = STRING_LIST_INIT_NODUP;
struct rename_info rename;
int i, refspec_updated = 0;
@@ -742,8 +747,8 @@ static int mv(int argc, const char **argv)
strbuf_reset(&buf3);
strbuf_addf(&buf3, "remote: renamed %s to %s",
item->string, buf.buf);
- if (create_symref(buf.buf, buf2.buf, buf3.buf))
- die(_("creating '%s' failed"), buf.buf);
+ if (create_symref(buf.buf, buf2.buf, buf3.buf, &err))
+ die(_("creating '%s' failed. %s"), buf.buf, err.buf);
}
return 0;
}
@@ -1260,6 +1265,7 @@ static int set_head(int argc, const char **argv)
{
int i, opt_a = 0, opt_d = 0, result = 0;
struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
+ struct strbuf err = STRBUF_INIT;
char *head_name = NULL;
struct option options[] = {
@@ -1302,8 +1308,12 @@ static int set_head(int argc, const char **argv)
/* make sure it's valid */
if (!ref_exists(buf2.buf))
result |= error(_("Not a valid ref: %s"), buf2.buf);
- else if (create_symref(buf.buf, buf2.buf, "remote set-head"))
- result |= error(_("Could not setup %s"), buf.buf);
+ else if (create_symref(buf.buf, buf2.buf, "remote set-head",
+ &err)) {
+ error(_("Could not setup %s. %s"), buf.buf, err.buf);
+ strbuf_release(&err);
+ result = -1;
+ }
if (opt_a)
printf("%s/HEAD set to %s\n", argv[0], head_name);
free(head_name);
diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index 29fb3f1..f9ca959 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -35,6 +35,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
{
int quiet = 0, delete = 0, shorten = 0, ret = 0;
const char *msg = NULL;
+ struct strbuf err = STRBUF_INIT;
struct option options[] = {
OPT__QUIET(&quiet,
N_("suppress error message for non-symbolic (detached) refs")),
@@ -67,7 +68,10 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
if (!strcmp(argv[0], "HEAD") &&
!starts_with(argv[1], "refs/"))
die("Refusing to point HEAD outside of refs/");
- create_symref(argv[0], argv[1], msg);
+ if (create_symref(argv[0], argv[1], msg, &err)) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ }
break;
default:
usage_with_options(git_symbolic_ref_usage, options);
diff --git a/cache.h b/cache.h
index 61e61af..3443da7 100644
--- a/cache.h
+++ b/cache.h
@@ -1026,7 +1026,6 @@ extern int get_sha1_mb(const char *str, unsigned char *sha1);
*/
extern int refname_match(const char *abbrev_name, const char *full_name);
-extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg);
extern int validate_headref(const char *ref);
extern int base_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);
diff --git a/refs.c b/refs.c
index 04ed571..e63004d 100644
--- a/refs.c
+++ b/refs.c
@@ -3109,20 +3109,22 @@ static int write_ref_sha1(struct ref_lock *lock,
}
int create_symref(const char *ref_target, const char *refs_heads_master,
- const char *logmsg)
+ const char *logmsg, struct strbuf *err)
{
const char *lockpath;
char ref[1000];
int fd, len, written;
char *git_HEAD = git_pathdup("%s", ref_target);
unsigned char old_sha1[20], new_sha1[20];
- struct strbuf err = STRBUF_INIT;
if (logmsg && read_ref(ref_target, old_sha1))
hashclr(old_sha1);
- if (safe_create_leading_directories(git_HEAD) < 0)
- return error("unable to create directory for %s", git_HEAD);
+ if (safe_create_leading_directories(git_HEAD) < 0) {
+ strbuf_addf(err, "unable to create directory for %s.",
+ git_HEAD);
+ return -1;
+ }
#ifndef NO_SYMLINK_HEAD
if (prefer_symlink_refs) {
@@ -3135,26 +3137,29 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
len = snprintf(ref, sizeof(ref), "ref: %s\n", refs_heads_master);
if (sizeof(ref) <= len) {
- error("refname too long: %s", refs_heads_master);
+ strbuf_addf(err, "refname too long: %s", refs_heads_master);
goto error_free_return;
}
lockpath = mkpath("%s.lock", git_HEAD);
fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666);
if (fd < 0) {
- error("Unable to open %s for writing", lockpath);
+ strbuf_addf(err, "Unable to open %s for writing. %s", lockpath,
+ strerror(errno));
goto error_free_return;
}
written = write_in_full(fd, ref, len);
if (close(fd) != 0 || written != len) {
- error("Unable to write to %s", lockpath);
+ strbuf_addf(err, "Unable to write to %s. %s", lockpath,
+ strerror(errno));
goto error_unlink_return;
}
if (rename(lockpath, git_HEAD) < 0) {
- error("Unable to create %s", git_HEAD);
+ strbuf_addf(err, "Unable to create %s. %s", git_HEAD,
+ strerror(errno));
goto error_unlink_return;
}
if (adjust_shared_perm(git_HEAD)) {
- error("Unable to fix permissions on %s", lockpath);
+ strbuf_addf(err, "Unable to fix permissions on %s", lockpath);
error_unlink_return:
unlink_or_warn(lockpath);
error_free_return:
@@ -3166,10 +3171,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
done:
#endif
if (logmsg && !read_ref(refs_heads_master, new_sha1) &&
- log_ref_write(ref_target, old_sha1, new_sha1, logmsg, &err))
- error("%s", err.buf);
+ log_ref_write(ref_target, old_sha1, new_sha1, logmsg, err)) {
+ error("%s", err->buf);
+ strbuf_release(err);
+ }
- strbuf_release(&err);
free(git_HEAD);
return 0;
}
diff --git a/refs.h b/refs.h
index 78af3a6..40fd294 100644
--- a/refs.h
+++ b/refs.h
@@ -116,6 +116,7 @@ static inline const char *has_glob_specials(const char *pattern)
/* can be used to learn about broken ref and symref */
extern int for_each_rawref(each_ref_fn, void *);
+extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg, struct strbuf *err);
extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);
extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct string_list *refnames);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 10+ messages in thread