[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] Add a flag to push atomically

[Stefan Beller]

This patch series adds a flag to git push to update the remote refs atomically.

It was part of a longer patch series[1].
This series applies on top of origin/mh/reflog-expire
It can also be found at [2].

Change since picking the series up from Ronnie.
  * other anchor point (i.e. where the series applies)
  * more tests for this feature, specially testing failures
  * drop the patch to rename ref_transaction_* to transaction_*_ref
  * slight rewording of the additional documentation

[1] http://www.spinics.net/lists/git/msg241214.html
[2] https://github.com/stefanbeller/git/tree/atomic-push-v1

Ronnie Sahlberg (4):
  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
  push.c: add an --atomic-push argument

Stefan Beller (1):
  t5543-atomic-push.sh: add basic tests for atomic pushes

 Documentation/git-push.txt                        |   8 +-
 Documentation/git-send-pack.txt                   |   7 +-
 Documentation/technical/protocol-capabilities.txt |  12 +-
 builtin/push.c                                    |   2 +
 builtin/receive-pack.c                            |  79 +++++++--
 builtin/send-pack.c                               |   6 +-
 remote.h                                          |   3 +-
 send-pack.c                                       |  45 +++++-
 send-pack.h                                       |   1 +
 t/t5543-atomic-push.sh                            | 185 ++++++++++++++++++++++
 transport.c                                       |   5 +
 transport.h                                       |   1 +
 12 files changed, 327 insertions(+), 27 deletions(-)
 create mode 100755 t/t5543-atomic-push.sh

-- 
2.2.0.33.gc2219e3.dirty

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

[Stefan Beller]

From: Ronnie Sahlberg <sahlberg@google.com>

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>
Signed-off-by: Stefan Beller <sbeller@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 6d5424c..763120c 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 32fc540..0c642ab 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)
@@ -1179,6 +1181,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.2.0.33.gc2219e3.dirty

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

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> From: Ronnie Sahlberg <sahlberg@google.com>
>
> 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.

Hmph, am I reading the patch to send-pack.c correctly?

It detects if the other side supports the capability and leaves it
in atomic_push_supported variable for later use, and also requests
the feature to be activated when atomic_push is set, but I see no
logic to link these two together, e.g. error out when atomic_push
is true and atomic_push_supported is false (or turn it off with a
warning, or whatever).

> 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());

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

[Stefan Beller]

On Mon, Dec 15, 2014 at 12:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Hmph, am I reading the patch to send-pack.c correctly?
>
> It detects if the other side supports the capability and leaves it
> in atomic_push_supported variable for later use, and also requests
> the feature to be activated when atomic_push is set, but I see no
> logic to link these two together, e.g. error out when atomic_push
> is true and atomic_push_supported is false (or turn it off with a
> warning, or whatever).


This is what you mean by

>
>> +     if (args->use_atomic_push && !atomic_push_supported) {
>> +             fprintf(stderr, "Server does not support atomic-push.");
>> +             return -1;
>> +     }
>
> This check logically belongs to the previous step, no?

from the next patch? If so it will be part of the next reroll.

Thanks,
Stefan

[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

[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.528.gd0e7a84