All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: git@vger.kernel.org
Cc: mhagger@alum.mit.edu, jrnieder@gmail.com, gitster@pobox.com,
	ronniesahlberg@gmail.com, Ronnie Sahlberg <sahlberg@google.com>,
	Stefan Beller <sbeller@google.com>
Subject: [PATCH 2/5] send-pack.c: add an --atomic-push command line argument
Date: Mon, 15 Dec 2014 11:56:05 -0800	[thread overview]
Message-ID: <1418673368-20785-3-git-send-email-sbeller@google.com> (raw)
In-Reply-To: <1418673368-20785-1-git-send-email-sbeller@google.com>

From: Ronnie Sahlberg <sahlberg@google.com>

This adds support to send-pack 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 doesn't send 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>
Signed-off-by: Stefan Beller <sbeller@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 70d38e4..e87481d 100644
--- a/transport.c
+++ b/transport.c
@@ -728,6 +728,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.2.0.33.gc2219e3.dirty

  parent reply	other threads:[~2014-12-15 19:56 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-15 19:56 [PATCH 0/5] Add a flag to push atomically Stefan Beller
2014-12-15 19:56 ` [PATCH 1/5] receive-pack.c: add protocol support to negotiate atomic-push Stefan Beller
2014-12-15 20:53   ` Junio C Hamano
2014-12-15 22:30     ` Stefan Beller
2014-12-15 19:56 ` Stefan Beller [this message]
2014-12-15 21:01   ` [PATCH 2/5] send-pack.c: add an --atomic-push command line argument Junio C Hamano
2014-12-15 19:56 ` [PATCH 3/5] receive-pack.c: use a single ref_transaction for atomic pushes Stefan Beller
2014-12-15 21:37   ` Junio C Hamano
2014-12-15 19:56 ` [PATCH 4/5] push.c: add an --atomic-push argument Stefan Beller
2014-12-15 21:50   ` Junio C Hamano
2014-12-15 19:56 ` [PATCH 5/5] t5543-atomic-push.sh: add basic tests for atomic pushes Stefan Beller
2014-12-15 22:29   ` Junio C Hamano
2014-12-15 22:33 ` [PATCH 0/5] Add a flag to push atomically Junio C Hamano
2014-12-16 18:49   ` [PATCHv2 1/6] receive-pack.c: add protocol support to negotiate atomic-push Stefan Beller
2014-12-16 18:49     ` [PATCHv2 2/6] send-pack: Invert the return value of ref_update_to_be_sent Stefan Beller
2014-12-16 19:14       ` Junio C Hamano
2014-12-16 18:49     ` [PATCHv2 3/6] send-pack.c: add --atomic command line argument Stefan Beller
2014-12-16 19:31       ` Junio C Hamano
2014-12-16 18:49     ` [PATCHv2 4/6] receive-pack.c: use a single ref_transaction for atomic pushes Stefan Beller
2014-12-16 19:29       ` Eric Sunshine
2014-12-16 20:30         ` Eric Sunshine
2014-12-16 19:35       ` Junio C Hamano
2014-12-16 18:49     ` [PATCHv2 5/6] push.c: add an --atomic-push argument Stefan Beller
2014-12-16 19:33       ` Eric Sunshine
2014-12-16 20:43         ` Junio C Hamano
2014-12-16 19:36       ` Junio C Hamano
2014-12-16 18:49     ` [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes Stefan Beller
2014-12-16 19:14       ` [PATCH] receive-pack: refuse all commands if one fails in atomic mode Stefan Beller
2014-12-16 20:32         ` Junio C Hamano
2014-12-16 19:37       ` [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes Eric Sunshine
2014-12-16 19:46       ` Junio C Hamano
2014-12-16 19:57         ` Stefan Beller
2014-12-16 20:46           ` Junio C Hamano
2014-12-16 20:51             ` Stefan Beller
2014-12-16 20:30       ` Junio C Hamano
2014-12-16 20:36         ` Stefan Beller
2014-12-16 19:05     ` [PATCHv2 1/6] receive-pack.c: add protocol support to negotiate atomic-push Junio C Hamano
2014-12-17 18:32   ` [PATCHv3 0/6] atomic pushes Stefan Beller
2014-12-17 18:32     ` [PATCHv3 1/6] receive-pack.c: add protocol support to negotiate atomic Stefan Beller
2014-12-19  1:05       ` Eric Sunshine
2014-12-17 18:32     ` [PATCHv3 2/6] send-pack: Rename ref_update_to_be_sent to check_to_send_update Stefan Beller
2014-12-17 22:53       ` Junio C Hamano
2014-12-17 18:32     ` [PATCHv3 3/6] send-pack.c: add --atomic command line argument Stefan Beller
2014-12-17 23:14       ` Junio C Hamano
2014-12-19  1:22       ` Eric Sunshine
2014-12-17 18:32     ` [PATCHv3 4/6] receive-pack.c: use a single ref_transaction for atomic pushes Stefan Beller
2014-12-17 23:26       ` Junio C Hamano
2014-12-17 23:58         ` Stefan Beller
2014-12-18 17:02           ` Junio C Hamano
2014-12-18 17:45             ` [PATCHv4 " Stefan Beller
2014-12-18 22:26               ` Eric Sunshine
2014-12-19  0:22                 ` [PATCHv5 " Stefan Beller
2014-12-19 10:14                   ` Eric Sunshine
2014-12-17 18:32     ` [PATCHv3 5/6] push.c: add an --atomic argument Stefan Beller
2014-12-19  1:29       ` Eric Sunshine
2014-12-17 18:32     ` [PATCHv3 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes Stefan Beller
  -- strict thread matches above, loose matches on Subject: below --
2014-08-19 16:24 [PATCH 0/5] ref-transactions-send-pack Ronnie Sahlberg
2014-08-19 16:24 ` [PATCH 2/5] send-pack.c: add an --atomic-push command line argument Ronnie Sahlberg
2014-07-31 21:39 [PATCH 0/5] ref-transactions-send-pack Ronnie Sahlberg
2014-07-31 21:39 ` [PATCH 2/5] send-pack.c: add an --atomic-push command line argument Ronnie Sahlberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1418673368-20785-3-git-send-email-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=mhagger@alum.mit.edu \
    --cc=ronniesahlberg@gmail.com \
    --cc=sahlberg@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.