[PATCH 0/5] ref-transactions-send-pack

[PATCH 0/5] ref-transactions-send-pack

[Ronnie Sahlberg]

List,

This small patch series adds atomic-push support for pushes.
By default git will use the old style non-atomic updates for pushes,
as not to cause disruption in client scripts that may depend on that
behaviour.

Command line arguments are introduced to allow the client side to request/
negotiate atomic pushes if the remote repo supports it.
There is also a new configuration variable where a repo can set that it
wants all pushes to become atomic whether the client requests it or not.

This patch series is called ref-transactions-send-pack and depends on/is built
ontop of the series called ref-transactions-req-strbuf-err

Ronnie Sahlberg (5):
  receive-pack.c: add protocol support to negotiate atomic-push
  send-pack.c: add an --atomic-push command line argument
  receive-pack.c: use a single transaction when atomic-push is
    negotiated
  receive-pack.c: add receive.atomicpush configuration option
  push.c: add an --atomic-push argument

 Documentation/config.txt                          |  5 ++
 Documentation/git-push.txt                        |  7 ++-
 Documentation/git-send-pack.txt                   |  7 ++-
 Documentation/technical/protocol-capabilities.txt |  7 +++
 builtin/push.c                                    |  2 +
 builtin/receive-pack.c                            | 66 ++++++++++++++++++-----
 builtin/send-pack.c                               |  6 ++-
 send-pack.c                                       | 18 +++++--
 send-pack.h                                       |  1 +
 transport.c                                       |  1 +
 transport.h                                       |  1 +
 11 files changed, 103 insertions(+), 18 deletions(-)

-- 
2.0.1.556.ge8f7cba.dirty

[PATCH 1/5] receive-pack.c: add protocol support to negotiate atomic-push

[Ronnie Sahlberg]

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
 builtin/receive-pack.c |  6 +++++-
 send-pack.c            | 12 +++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0565b94..f6b20cb 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -36,6 +36,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;
@@ -142,7 +143,8 @@ static void show_ref(const char *path, const unsigned char *sha1)
 	else
 		packet_write(1, "%s %s%c%s%s agent=%s\n",
 			     sha1_to_hex(sha1), path, 0,
-			     " report-status delete-refs side-band-64k quiet",
+			     " report-status delete-refs side-band-64k quiet"
+			     " atomic-push",
 			     prefer_ofs_delta ? " ofs-delta" : "",
 			     git_user_agent_sanitized());
 	sent_capabilities = 1;
@@ -892,6 +894,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;
 		}
 		cmd = xcalloc(1, sizeof(struct command) + len - 80);
 		hashcpy(cmd->old_sha1, old_sha1);
diff --git a/send-pack.c b/send-pack.c
index 6129b0f..f91b8d9 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -205,6 +205,7 @@ 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;
 	unsigned cmds_sent = 0;
 	int ret;
 	struct async demux;
@@ -224,6 +225,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 (!remote_refs) {
 		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
@@ -269,17 +272,20 @@ int send_pack(struct send_pack_args *args,
 			char *old_hex = sha1_to_hex(ref->old_sha1);
 			char *new_hex = sha1_to_hex(ref->new_sha1);
 			int quiet = quiet_supported && (args->quiet || !args->progress);
+			int atomic_push = atomic_push_supported;
 
 			if (!cmds_sent && (status_report || use_sideband ||
-					   quiet || agent_supported)) {
+					   quiet || agent_supported ||
+					   atomic_push)) {
 				packet_buf_write(&req_buf,
-						 "%s %s %s%c%s%s%s%s%s",
+						 "%s %s %s%c%s%s%s%s%s%s",
 						 old_hex, new_hex, ref->name, 0,
 						 status_report ? " report-status" : "",
 						 use_sideband ? " side-band-64k" : "",
 						 quiet ? " quiet" : "",
 						 agent_supported ? " agent=" : "",
-						 agent_supported ? git_user_agent_sanitized() : ""
+						 agent_supported ? git_user_agent_sanitized() : "",
+						 atomic_push ? " atomic-push" : ""
 						);
 			}
 			else
-- 
2.0.1.556.ge8f7cba.dirty

[PATCH 2/5] send-pack.c: add an --atomic-push command line argument

[Ronnie Sahlberg]

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
 Documentation/git-send-pack.txt | 7 ++++++-
 builtin/send-pack.c             | 6 +++++-
 send-pack.c                     | 8 +++++++-
 send-pack.h                     | 1 +
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index dc3a568..4ee2ca1 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
 -----------
@@ -52,6 +52,11 @@ OPTIONS
 	Send a "thin" pack, which records objects in deltified form based
 	on objects not included in the pack to reduce network traffic.
 
+--atomic-push::
+	With atomic-push all refs are updated in one single atomic transaction.
+	This means that if any of the refs fails 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 f420b74..78e7d8f 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;
@@ -165,6 +165,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/send-pack.c b/send-pack.c
index f91b8d9..66f3724 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -228,6 +228,11 @@ int send_pack(struct send_pack_args *args,
 	if (server_supports("atomic-push"))
 		atomic_push_supported = 1;
 
+	if (args->use_atomic_push && !atomic_push_supported) {
+		fprintf(stderr, "Server does not support atomic-push.");
+		return -1;
+	}
+
 	if (!remote_refs) {
 		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
 			"Perhaps you should specify a branch such as 'master'.\n");
@@ -272,7 +277,8 @@ int send_pack(struct send_pack_args *args,
 			char *old_hex = sha1_to_hex(ref->old_sha1);
 			char *new_hex = sha1_to_hex(ref->new_sha1);
 			int quiet = quiet_supported && (args->quiet || !args->progress);
-			int atomic_push = atomic_push_supported;
+			int atomic_push = atomic_push_supported &&
+				args->use_atomic_push;
 
 			if (!cmds_sent && (status_report || use_sideband ||
 					   quiet || agent_supported ||
diff --git a/send-pack.h b/send-pack.h
index 8e84392..0374ed8 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -10,6 +10,7 @@ struct send_pack_args {
 		force_update:1,
 		use_thin_pack:1,
 		use_ofs_delta:1,
+		use_atomic_push:1,
 		dry_run:1,
 		stateless_rpc:1;
 };
-- 
2.0.1.556.ge8f7cba.dirty

[PATCH 3/5] receive-pack.c: use a single transaction when atomic-push is negotiated

[Ronnie Sahlberg]

Update receive-pack to use an atomic transaction IFF the client negotiated
that it wanted atomic-push.
This leaves the default behaviour 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 behaviour 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 behaviour 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 behaviour 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>
---
 Documentation/technical/protocol-capabilities.txt |  7 +++
 builtin/receive-pack.c                            | 55 ++++++++++++++++++-----
 2 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index e174343..93d99c5 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -250,3 +250,10 @@ allow-tip-sha1-in-want
 If the upload-pack server advertises this capability, fetch-pack may
 send "want" lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.
+
+atomic-push
+-----------
+
+If the receive-pack server advertises the 'atomic-push' capability, it means
+that it is capable of accepting atomic pushes. An atomic push is a push
+where the ref updates are done as a single atomic transaction.
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f6b20cb..47f778d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -47,6 +47,8 @@ static void *head_name_to_free;
 static int sent_capabilities;
 static int shallow_update;
 static const char *alt_shallow_file;
+struct strbuf err = STRBUF_INIT;
+struct ref_transaction *transaction;
 
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
@@ -577,26 +579,38 @@ static char *update(struct command *cmd, struct shallow_info *si)
 		return NULL; /* good */
 	}
 	else {
-		struct strbuf err = STRBUF_INIT;
-		struct ref_transaction *transaction;
-
 		if (shallow_update && si->shallow_ref[cmd->index] &&
 		    update_shallow_ref(cmd, si))
 			return xstrdup("shallow error");
-
-		transaction = transaction_begin(&err);
-		if (!transaction ||
-		    transaction_update_sha1(transaction, namespaced_name,
+		if (!use_atomic_push) {
+			transaction = transaction_begin(&err);
+			if (!transaction) {
+				char *str = xstrdup(err.buf);
+
+				strbuf_release(&err);
+				transaction_free(transaction);
+				rp_error("%s", str);
+				return str;
+			}
+		}
+		if (transaction_update_sha1(transaction, namespaced_name,
 					    new_sha1, old_sha1, 0, 1, "push",
-					    &err) ||
-		    transaction_commit(transaction, &err)) {
-			char *str = strbuf_detach(&err, NULL);
-			transaction_free(transaction);
+					    &err)) {
+			char *str = xstrdup(err.buf);
 
+			strbuf_release(&err);
+			transaction_free(transaction);
 			rp_error("%s", str);
 			return str;
 		}
+		if (!use_atomic_push && transaction_commit(transaction, &err)) {
+			char *str = xstrdup(err.buf);
 
+			strbuf_release(&err);
+			transaction_free(transaction);
+			rp_error("%s", str);
+			return str;
+		}
 		transaction_free(transaction);
 		strbuf_release(&err);
 		return NULL; /* good */
@@ -810,6 +824,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))
@@ -848,6 +872,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 "
@@ -1250,5 +1282,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	sha1_array_clear(&shallow);
 	sha1_array_clear(&ref);
 	free_commands(commands);
+	strbuf_release(&err);
 	return 0;
 }
-- 
2.0.1.556.ge8f7cba.dirty

[PATCH 4/5] receive-pack.c: add receive.atomicpush configuration option

[Ronnie Sahlberg]

Add a configuration argument to the receive side to force atomic pushes
for all pushes to the repo.

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
 Documentation/config.txt | 5 +++++
 builtin/receive-pack.c   | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1d718bd..75ce157 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2038,6 +2038,11 @@ rebase.autostash::
 	successful rebase might result in non-trivial conflicts.
 	Defaults to false.
 
+receive.atomicpush::
+	By default, git-receive-pack will only use atomic ref transactions
+	if the client negotiates it. When set to true, git-receive-pack
+	will force atomic ref updates for all client pushes.
+
 receive.autogc::
 	By default, git-receive-pack will run "git-gc --auto" after
 	receiving data from git-push and updating refs.  You can stop
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 47f778d..9fa637a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -132,6 +132,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (strcmp(var, "receive.atomicpush") == 0) {
+		use_atomic_push = git_config_bool(var, value);
+		return 0;
+	}
+
 	return git_default_config(var, value, cb);
 }
 
-- 
2.0.1.556.ge8f7cba.dirty

[PATCH 5/5] push.c: add an --atomic-push argument

[Ronnie Sahlberg]

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 21cd455..b80b0ac 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]
 	   [--force-with-lease[=<refname>[:<expect>]]]
 	   [--no-verify] [<repository> [<refspec>...]]
@@ -129,6 +129,11 @@ already exists on the remote side.
 	from the remote but are pointing at commit-ish that are
 	reachable from the refs being pushed.
 
+--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 f8dfea4..f37390c 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, "no-verify", &flags, N_("bypass pre-push hook"), TRANSPORT_PUSH_NO_HOOK),
 		OPT_BIT(0, "follow-tags", &flags, N_("push missing but relevant tags"),
 			TRANSPORT_PUSH_FOLLOW_TAGS),
+		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 a3b7f48..ab5f553 100644
--- a/transport.c
+++ b/transport.c
@@ -837,6 +837,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
 	args.progress = transport->progress;
 	args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
 	args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);
+	args.use_atomic_push = !!(flags & TRANSPORT_ATOMIC_PUSH);
 
 	ret = send_pack(&args, data->fd, data->conn, remote_refs,
 			&data->extra_have);
diff --git a/transport.h b/transport.h
index 02ea248..407d641 100644
--- a/transport.h
+++ b/transport.h
@@ -123,6 +123,7 @@ struct transport {
 #define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256
 #define TRANSPORT_PUSH_NO_HOOK 512
 #define TRANSPORT_PUSH_FOLLOW_TAGS 1024
+#define TRANSPORT_ATOMIC_PUSH 2048
 
 #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
 #define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)
-- 
2.0.1.556.ge8f7cba.dirty

[PATCH 0/5] ref-transactions-send-pack

[Ronnie Sahlberg]

List,

This small patch series adds atomic-push support to for pushes.
By default git will use the old style non-atomic updates for pushes,
as not to cause disruption in client scripts that may depend on that
behaviour.

Command line arguments are introduced to allow the client side to request/
negotiate atomic pushes if the remote repo supports it.
There is also a new configuration variable where a repo can set that it
wants all pushes to become atomic whether the client requests it or not.

This patch series is called ref-transactions-send-pack and depends on/is built
ontop of the series called ref-transactions-req-strbuf-err


Ronnie Sahlberg (5):
  receive-pack.c: add protocol support to negotiate atomic-push
  send-pack.c: add an --atomic-push command line argument
  receive-pack.c: use a single transaction when atomic-push is
    negotiated
  receive-pack.c: add receive.atomicpush configuration option
  push.c: add an --atomic-push argument

 Documentation/config.txt        |  5 ++++
 Documentation/git-push.txt      |  7 ++++-
 Documentation/git-send-pack.txt |  7 ++++-
 builtin/push.c                  |  2 ++
 builtin/receive-pack.c          | 66 +++++++++++++++++++++++++++++++++--------
 builtin/send-pack.c             |  6 +++-
 send-pack.c                     | 18 +++++++++--
 send-pack.h                     |  1 +
 transport.c                     |  1 +
 transport.h                     |  1 +
 10 files changed, 96 insertions(+), 18 deletions(-)

-- 
2.0.1.528.gd0e7a84

Re: [PATCH 0/5] ref-transactions-send-pack

[Ronnie Sahlberg]

Ping

On Thu, Jul 31, 2014 at 2:39 PM, Ronnie Sahlberg <sahlberg@google.com> wrote:
> List,
>
> This small patch series adds atomic-push support to for pushes.
> By default git will use the old style non-atomic updates for pushes,
> as not to cause disruption in client scripts that may depend on that
> behaviour.
>
> Command line arguments are introduced to allow the client side to request/
> negotiate atomic pushes if the remote repo supports it.
> There is also a new configuration variable where a repo can set that it
> wants all pushes to become atomic whether the client requests it or not.
>
> This patch series is called ref-transactions-send-pack and depends on/is built
> ontop of the series called ref-transactions-req-strbuf-err
>
>
> Ronnie Sahlberg (5):
>   receive-pack.c: add protocol support to negotiate atomic-push
>   send-pack.c: add an --atomic-push command line argument
>   receive-pack.c: use a single transaction when atomic-push is
>     negotiated
>   receive-pack.c: add receive.atomicpush configuration option
>   push.c: add an --atomic-push argument
>
>  Documentation/config.txt        |  5 ++++
>  Documentation/git-push.txt      |  7 ++++-
>  Documentation/git-send-pack.txt |  7 ++++-
>  builtin/push.c                  |  2 ++
>  builtin/receive-pack.c          | 66 +++++++++++++++++++++++++++++++++--------
>  builtin/send-pack.c             |  6 +++-
>  send-pack.c                     | 18 +++++++++--
>  send-pack.h                     |  1 +
>  transport.c                     |  1 +
>  transport.h                     |  1 +
>  10 files changed, 96 insertions(+), 18 deletions(-)
>
> --
> 2.0.1.528.gd0e7a84
>