git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>
Subject: [PATCH 2/4] push: teach --base for ssh:// and file://
Date: Mon,  2 Nov 2020 16:26:11 -0800	[thread overview]
Message-ID: <148e39960a2185d2355cdfe34f8856e708fb1b80.1604362701.git.jonathantanmy@google.com> (raw)
In-Reply-To: <cover.1604362701.git.jonathantanmy@google.com>

For the ssh:// and file:// transports, teach push the "--base"
parameter, which indicates an ancestor of the commits to be pushed that
is believed to be known by the server.

If specified, it will be communicated to receive-pack through an Extra
Parameter. If the repository served by receive-pack indeed has that
object, receive-pack will send an abbreviated ref advertisement
containing only that object instead of the usual ref advertisement,
reducing the size of the ref advertisement and, potentially, the size of
the packfile to be sent.

This is supported both on protocol v0 and v1. (Protocol v1 is the same
as protocol v0, except that the client sends "version=1" as an Extra
Parameter and the server responds with "version 1".)

Support for http(s):// will be added in a subsequent patch.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 builtin/fetch-pack.c   |  2 +-
 builtin/push.c         | 12 +++++--
 builtin/receive-pack.c | 46 +++++++++++++++++++++---
 builtin/send-pack.c    |  2 +-
 builtin/upload-pack.c  |  2 +-
 connect.c              |  8 ++++-
 connect.h              |  4 ++-
 http-backend.c         |  2 +-
 protocol.c             |  5 ++-
 protocol.h             |  5 ++-
 t/t5700-protocol-v1.sh | 80 ++++++++++++++++++++++++++++++++++++++++++
 transport.c            | 14 ++++++--
 transport.h            |  9 +++++
 13 files changed, 175 insertions(+), 16 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 58b7c1fbdc..9452969319 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -207,7 +207,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 		if (args.diag_url)
 			flags |= CONNECT_DIAG_URL;
 		conn = git_connect(fd, dest, args.uploadpack,
-				   flags);
+				   flags, NULL);
 		if (!conn)
 			return args.diag_url ? 0 : 1;
 	}
diff --git a/builtin/push.c b/builtin/push.c
index 6da3a8e5d3..a9f3278a24 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -381,7 +381,8 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
 
 static int do_push(int flags,
 		   const struct string_list *push_options,
-		   struct remote *remote)
+		   struct remote *remote,
+		   const char *push_base)
 {
 	int i, errs;
 	const char **url;
@@ -405,6 +406,8 @@ static int do_push(int flags,
 				transport_get(remote, url[i]);
 			if (flags & TRANSPORT_PUSH_OPTIONS)
 				transport->push_options = push_options;
+			if (push_base)
+				transport_set_option(transport, TRANS_OPT_PUSH_BASE, push_base);
 			if (push_with_options(transport, push_refspec, flags))
 				errs++;
 		}
@@ -413,6 +416,8 @@ static int do_push(int flags,
 			transport_get(remote, NULL);
 		if (flags & TRANSPORT_PUSH_OPTIONS)
 			transport->push_options = push_options;
+		if (push_base)
+			transport_set_option(transport, TRANS_OPT_PUSH_BASE, push_base);
 		if (push_with_options(transport, push_refspec, flags))
 			errs++;
 	}
@@ -526,6 +531,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	struct string_list *push_options;
 	const struct string_list_item *item;
 	struct remote *remote;
+	const char *push_base = NULL;
 
 	struct option options[] = {
 		OPT__VERBOSITY(&verbosity),
@@ -562,6 +568,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 				TRANSPORT_FAMILY_IPV4),
 		OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
 				TRANSPORT_FAMILY_IPV6),
+		OPT_STRING(0, "base", &push_base, N_("revision"),
+			   N_("ancestor of commits to be pushed that is believed to be known by the server")),
 		OPT_END()
 	};
 
@@ -629,7 +637,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		if (strchr(item->string, '\n'))
 			die(_("push options must not have new line characters"));
 
-	rc = do_push(flags, push_options, remote);
+	rc = do_push(flags, push_options, remote, push_base);
 	string_list_clear(&push_options_cmdline, 0);
 	string_list_clear(&push_options_config, 0);
 	if (rc == -1)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index bb9909c52e..274f39c944 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -313,13 +313,38 @@ static void show_one_alternate_ref(const struct object_id *oid,
 	show_ref(".have", oid);
 }
 
-static void write_head_info(void)
+struct check_connected_for_one_args {
+	struct object_id *base;
+	unsigned already_emitted : 1;
+};
+static int check_connected_for_one(void *args, struct object_id *oid)
+{
+	struct check_connected_for_one_args *a = args;
+
+	if (a->already_emitted)
+		return -1;
+	oidcpy(oid, a->base);
+	a->already_emitted = 1;
+	return 0;
+}
+
+static void write_head_info(struct object_id *base)
 {
 	static struct oidset seen = OIDSET_INIT;
 
+	if (base) {
+		struct check_connected_for_one_args args = {base};
+
+		if (!check_connected(check_connected_for_one, &args, NULL)) {
+			show_ref(".have", base);
+			goto refs_shown;
+		}
+	}
+
 	for_each_ref(show_ref_cb, &seen);
 	for_each_alternate_ref(show_one_alternate_ref, &seen);
 	oidset_clear(&seen);
+refs_shown:
 	if (!sent_capabilities)
 		show_ref("capabilities^{}", &null_oid);
 
@@ -2417,6 +2442,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	struct oid_array ref = OID_ARRAY_INIT;
 	struct shallow_info si;
 	struct packet_reader reader;
+	struct strbuf base_sb = STRBUF_INIT;
 
 	struct option options[] = {
 		OPT__QUIET(&quiet, N_("quiet")),
@@ -2451,7 +2477,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	else if (0 <= receive_unpack_limit)
 		unpack_limit = receive_unpack_limit;
 
-	switch (determine_protocol_version_server()) {
+	switch (determine_protocol_version_server(&base_sb)) {
 	case protocol_v2:
 		/*
 		 * push support for protocol v2 has not been implemented yet,
@@ -2474,10 +2500,21 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	}
 
 	if (advertise_refs || !stateless_rpc) {
-		write_head_info();
+		if (base_sb.len) {
+			struct object_id oid;
+			const char *p;
+
+			if (parse_oid_hex(base_sb.buf, &oid, &p) || *p != '\0')
+				die("invalid base");
+			write_head_info(&oid);
+		} else {
+			write_head_info(NULL);
+		}
 	}
-	if (advertise_refs)
+	if (advertise_refs) {
+		strbuf_release(&base_sb);
 		return 0;
+	}
 
 	packet_reader_init(&reader, 0, NULL, 0,
 			   PACKET_READ_CHOMP_NEWLINE |
@@ -2540,6 +2577,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	}
 	if (use_sideband)
 		packet_flush(1);
+	strbuf_release(&base_sb);
 	oid_array_clear(&shallow);
 	oid_array_clear(&ref);
 	free((void *)push_cert_nonce);
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 7af148d733..1557db7a42 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -262,7 +262,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 		fd[1] = 1;
 	} else {
 		conn = git_connect(fd, dest, receivepack,
-			args.verbose ? CONNECT_VERBOSE : 0);
+			args.verbose ? CONNECT_VERBOSE : 0, NULL);
 	}
 
 	packet_reader_init(&reader, fd[0], NULL, 0,
diff --git a/builtin/upload-pack.c b/builtin/upload-pack.c
index 6da8fa2607..32aaa7742c 100644
--- a/builtin/upload-pack.c
+++ b/builtin/upload-pack.c
@@ -48,7 +48,7 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
 	if (!enter_repo(dir, strict))
 		die("'%s' does not appear to be a git repository", dir);
 
-	switch (determine_protocol_version_server()) {
+	switch (determine_protocol_version_server(NULL)) {
 	case protocol_v2:
 		serve_opts.advertise_capabilities = opts.advertise_refs;
 		serve_opts.stateless_rpc = opts.stateless_rpc;
diff --git a/connect.c b/connect.c
index 5221f1764c..fa5b7ea886 100644
--- a/connect.c
+++ b/connect.c
@@ -1318,7 +1318,8 @@ static void fill_ssh_args(struct child_process *conn, const char *ssh_host,
  * the connection failed).
  */
 struct child_process *git_connect(int fd[2], const char *url,
-				  const char *prog, int flags)
+				  const char *prog, int flags,
+				  const char *other_extra_parameters)
 {
 	char *hostandport, *path;
 	struct child_process *conn;
@@ -1369,6 +1370,11 @@ struct child_process *git_connect(int fd[2], const char *url,
 
 		if (version > 0)
 			strbuf_addf(&extra_parameters, "version=%d", version);
+		if (other_extra_parameters) {
+			if (extra_parameters.len)
+				strbuf_addch(&extra_parameters, ':');
+			strbuf_addstr(&extra_parameters, other_extra_parameters);
+		}
 
 		conn->use_shell = 1;
 		conn->in = conn->out = -1;
diff --git a/connect.h b/connect.h
index c53586e929..6d502d4224 100644
--- a/connect.h
+++ b/connect.h
@@ -7,7 +7,9 @@
 #define CONNECT_DIAG_URL      (1u << 1)
 #define CONNECT_IPV4          (1u << 2)
 #define CONNECT_IPV6          (1u << 3)
-struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
+struct child_process *git_connect(int fd[2], const char *url,
+				  const char *prog, int flags,
+				  const char *other_extra_parameters);
 int finish_connect(struct child_process *conn);
 int git_connection_is_socket(struct child_process *conn);
 int server_supports(const char *feature);
diff --git a/http-backend.c b/http-backend.c
index a03b4bae22..7cd684c8b5 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -544,7 +544,7 @@ static void get_info_refs(struct strbuf *hdr, char *arg)
 		end_headers(hdr);
 
 
-		if (determine_protocol_version_server() != protocol_v2) {
+		if (determine_protocol_version_server(NULL) != protocol_v2) {
 			packet_write_fmt(1, "# service=git-%s\n", svc->name);
 			packet_flush(1);
 		}
diff --git a/protocol.c b/protocol.c
index 052d7edbb9..ad653b7521 100644
--- a/protocol.c
+++ b/protocol.c
@@ -42,7 +42,7 @@ enum protocol_version get_protocol_version_config(void)
 	return protocol_v2;
 }
 
-enum protocol_version determine_protocol_version_server(void)
+enum protocol_version determine_protocol_version_server(struct strbuf *base_sb)
 {
 	const char *git_protocol = getenv(GIT_PROTOCOL_ENVIRONMENT);
 	enum protocol_version version = protocol_v0;
@@ -67,6 +67,9 @@ enum protocol_version determine_protocol_version_server(void)
 				v = parse_protocol_version(value);
 				if (v > version)
 					version = v;
+			} else if (skip_prefix(item->string, "base=", &value)) {
+				if (base_sb)
+					strbuf_addstr(base_sb, value);
 			}
 		}
 
diff --git a/protocol.h b/protocol.h
index cef1a4a01c..e418b92049 100644
--- a/protocol.h
+++ b/protocol.h
@@ -22,8 +22,11 @@ enum protocol_version get_protocol_version_config(void);
  * by setting appropriate values for the key 'version'.  If a client doesn't
  * request a particular protocol version, a default of 'protocol_v0' will be
  * used.
+ *
+ * If base_sb is not NULL and an extra parameter "base" is specified, this
+ * function will append its value to base_sb.
  */
-enum protocol_version determine_protocol_version_server(void);
+enum protocol_version determine_protocol_version_server(struct strbuf *base_sb);
 
 /*
  * Used by a client to determine which protocol version the server is speaking
diff --git a/t/t5700-protocol-v1.sh b/t/t5700-protocol-v1.sh
index 022901b9eb..22459d37f5 100755
--- a/t/t5700-protocol-v1.sh
+++ b/t/t5700-protocol-v1.sh
@@ -146,6 +146,56 @@ test_expect_success 'push with file:// using protocol v1' '
 	grep "push< version 1" log
 '
 
+test_expect_success 'push with file:// using protocol v1 and --base' '
+	test_commit -C file_child four &&
+	COMMON_HASH=$(git -C file_child rev-parse three) &&
+
+	# Push to another branch, as the target repository has the
+	# master branch checked out and we cannot push into it.
+	GIT_TRACE_PACKET=1 git -C file_child -c protocol.version=1 \
+		push --base=three origin HEAD:client_branch_four 2>log &&
+
+	# Server responded using protocol v1
+	grep "push< version 1" log &&
+	# Server advertised only the expected object
+	grep "$COMMON_HASH .have" log
+'
+
+test_expect_success 'push with file:// using protocol v0 and --base' '
+	test_commit -C file_child five &&
+	COMMON_HASH=$(git -C file_child rev-parse four) &&
+
+	# Push to another branch, as the target repository has the
+	# master branch checked out and we cannot push into it.
+	GIT_TRACE_PACKET=1 git -C file_child -c protocol.version=0 \
+		push --base=four origin HEAD:client_branch_five 2>log &&
+
+	# Server did not respond with any version
+	! grep "push< version" log &&
+	# Server advertised only the expected object
+	grep "$COMMON_HASH .have" log
+'
+
+test_expect_success 'push with invalid --base' '
+	test_commit -C file_child six &&
+
+	# Server does not have "six".
+	test_must_fail git -C file_child -c protocol.version=0 \
+		push --base=an_invalid_object origin HEAD:client_branch_six 2>log &&
+	grep "is not a valid object" log
+'
+
+test_expect_success 'push with --base that does not exist on server' '
+	COMMON_HASH=$(git -C file_child rev-parse six) &&
+
+	# The push still succeeds.
+	GIT_TRACE_PACKET=1 git -C file_child -c protocol.version=0 \
+		push --base=six origin HEAD:client_branch_six 2>log &&
+
+	# Server did not advertise "six", since it does not know it
+	! grep "$COMMON_HASH .have" log
+'
+
 # Test protocol v1 with 'ssh://' transport
 #
 test_expect_success 'setup ssh wrapper' '
@@ -226,6 +276,36 @@ test_expect_success 'push with ssh:// using protocol v1' '
 	grep "push< version 1" log
 '
 
+test_expect_success 'push with ssh:// using protocol v1 and --base' '
+	test_commit -C ssh_child four &&
+	COMMON_HASH=$(git -C ssh_child rev-parse three) &&
+
+	# Push to another branch, as the target repository has the
+	# master branch checked out and we cannot push into it.
+	GIT_TRACE_PACKET=1 git -C ssh_child -c protocol.version=1 \
+		push --base="$COMMON_HASH" origin HEAD:client_branch_four 2>log &&
+
+	# Server responded using protocol v1
+	grep "push< version 1" log &&
+	# Server advertised only the expected object
+	grep "$COMMON_HASH .have" log
+'
+
+test_expect_success 'push with ssh:// using protocol v0 and --base' '
+	test_commit -C ssh_child five &&
+	COMMON_HASH=$(git -C ssh_child rev-parse four) &&
+
+	# Push to another branch, as the target repository has the
+	# master branch checked out and we cannot push into it.
+	GIT_TRACE_PACKET=1 git -C ssh_child -c protocol.version=0 \
+		push --base="$COMMON_HASH" origin HEAD:client_branch_five 2>log &&
+
+	# Server did not respond with any version
+	! grep "push< version" log &&
+	# Server advertised only the expected object
+	grep "$COMMON_HASH .have" log
+'
+
 # Test protocol v1 with 'http://' transport
 #
 . "$TEST_DIRECTORY"/lib-httpd.sh
diff --git a/transport.c b/transport.c
index ffe2115845..531ca0a834 100644
--- a/transport.c
+++ b/transport.c
@@ -236,6 +236,10 @@ static int set_git_option(struct git_transport_options *opts,
 		list_objects_filter_die_if_populated(&opts->filter_options);
 		parse_list_objects_filter(&opts->filter_options, value);
 		return 0;
+	} else if (!strcmp(name, TRANS_OPT_PUSH_BASE)) {
+		if (get_oid(value, &opts->push_base))
+			die(_("transport: '%s' is not a valid object"), value);
+		return 0;
 	}
 	return 1;
 }
@@ -244,6 +248,7 @@ static int connect_setup(struct transport *transport, int for_push)
 {
 	struct git_transport_data *data = transport->data;
 	int flags = transport->verbose > 0 ? CONNECT_VERBOSE : 0;
+	char *extra_parameters = NULL;
 
 	if (data->conn)
 		return 0;
@@ -254,11 +259,16 @@ static int connect_setup(struct transport *transport, int for_push)
 	case TRANSPORT_FAMILY_IPV6: flags |= CONNECT_IPV6; break;
 	}
 
+	if (!is_null_oid(&data->options.push_base))
+		extra_parameters = xstrfmt("base=%s",
+					   oid_to_hex(&data->options.push_base));
+
 	data->conn = git_connect(data->fd, transport->url,
 				 for_push ? data->options.receivepack :
 				 data->options.uploadpack,
-				 flags);
+				 flags, extra_parameters);
 
+	free(extra_parameters);
 	return 0;
 }
 
@@ -815,7 +825,7 @@ static int connect_git(struct transport *transport, const char *name,
 {
 	struct git_transport_data *data = transport->data;
 	data->conn = git_connect(data->fd, transport->url,
-				 executable, 0);
+				 executable, 0, NULL);
 	fd[0] = data->fd[0];
 	fd[1] = data->fd[1];
 	return 0;
diff --git a/transport.h b/transport.h
index ca409ea1e4..ce51edd673 100644
--- a/transport.h
+++ b/transport.h
@@ -46,6 +46,12 @@ struct git_transport_options {
 	 * transport_set_option().
 	 */
 	struct oid_array *negotiation_tips;
+
+	/*
+	 * When pushing, if this is not a null OID, indicates an ancestor of
+	 * the commits to be pushed that is believed to be known by the server.
+	 */
+	struct object_id push_base;
 };
 
 enum transport_family {
@@ -208,6 +214,9 @@ void transport_check_allowed(const char *type);
 /* Request atomic (all-or-nothing) updates when pushing */
 #define TRANS_OPT_ATOMIC "atomic"
 
+/* See "push_base" in struct git_transport_options */
+#define TRANS_OPT_PUSH_BASE "push-base"
+
 /**
  * Returns 0 if the option was used, non-zero otherwise. Prints a
  * message to stderr if the option is not used.
-- 
2.29.1.341.ge80a0c044ae-goog


  parent reply	other threads:[~2020-11-03  0:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-03  0:26 [PATCH 0/4] "Push" protocol change proposal: user-specified base Jonathan Tan
2020-11-03  0:26 ` [PATCH 1/4] connect: refactor building of Extra Parameters Jonathan Tan
2020-11-03  0:26 ` Jonathan Tan [this message]
2020-11-03 10:23   ` [PATCH 2/4] push: teach --base for ssh:// and file:// SZEDER Gábor
2020-11-08 19:31     ` Junio C Hamano
2020-11-03 13:57   ` Derrick Stolee
2020-11-08 19:30   ` Junio C Hamano
2020-11-03  0:26 ` [PATCH 3/4] remote-curl: teach --base for http(s):// Jonathan Tan
2020-11-03  1:13   ` Junio C Hamano
2020-11-03  0:26 ` [PATCH 4/4] Doc: push with --base Jonathan Tan
2020-11-03  5:35   ` Jonathan Nieder
2020-11-03 15:18     ` Jeff King
2020-11-03 17:35       ` Junio C Hamano
2020-11-09 19:56         ` Jonathan Tan
2020-11-09 21:00           ` Derrick Stolee
2020-11-09 22:22             ` Jonathan Tan
2020-11-09 21:20           ` Junio C Hamano
2020-11-09 21:40             ` Jeff King
2020-11-09 22:47             ` Jonathan Tan
2020-11-03 13:53   ` Derrick Stolee
2020-11-03  0:46 ` [PATCH 0/4] "Push" protocol change proposal: user-specified base Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=148e39960a2185d2355cdfe34f8856e708fb1b80.1604362701.git.jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).