git@vger.kernel.org list mirror (unofficial, 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>,
	gitster@pobox.com, stolee@gmail.com
Subject: [PATCH v2 5/5] send-pack: support push negotiation
Date: Tue,  4 May 2021 14:16:02 -0700	[thread overview]
Message-ID: <d38a8b7d66efee74e327c4a7f4322e2c02a7495d.1620162764.git.jonathantanmy@google.com> (raw)
In-Reply-To: <cover.1620162764.git.jonathantanmy@google.com>

Teach Git the push.negotiate config variable.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config/push.txt |  7 ++++
 send-pack.c                   | 61 ++++++++++++++++++++++++++++++++---
 t/t5516-fetch-push.sh         | 35 ++++++++++++++++++++
 3 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/Documentation/config/push.txt b/Documentation/config/push.txt
index 21b256e0a4..f2667b2689 100644
--- a/Documentation/config/push.txt
+++ b/Documentation/config/push.txt
@@ -120,3 +120,10 @@ push.useForceIfIncludes::
 	`--force-if-includes` as an option to linkgit:git-push[1]
 	in the command line. Adding `--no-force-if-includes` at the
 	time of push overrides this configuration setting.
+
+push.negotiate::
+	If set to "true", attempt to reduce the size of the packfile
+	sent by rounds of negotiation in which the client and the
+	server attempt to find commits in common. If "false", Git will
+	rely solely on the server's ref advertisement to find commits
+	in common.
diff --git a/send-pack.c b/send-pack.c
index 5f215b13c7..9cb9f71650 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -56,7 +56,9 @@ static void feed_object(const struct object_id *oid, FILE *fh, int negative)
 /*
  * Make a pack stream and spit it out into file descriptor fd
  */
-static int pack_objects(int fd, struct ref *refs, struct oid_array *extra, struct send_pack_args *args)
+static int pack_objects(int fd, struct ref *refs, struct oid_array *advertised,
+			struct oid_array *negotiated,
+			struct send_pack_args *args)
 {
 	/*
 	 * The child becomes pack-objects --revs; we feed
@@ -94,8 +96,10 @@ static int pack_objects(int fd, struct ref *refs, struct oid_array *extra, struc
 	 * parameters by writing to the pipe.
 	 */
 	po_in = xfdopen(po.in, "w");
-	for (i = 0; i < extra->nr; i++)
-		feed_object(&extra->oid[i], po_in, 1);
+	for (i = 0; i < advertised->nr; i++)
+		feed_object(&advertised->oid[i], po_in, 1);
+	for (i = 0; i < negotiated->nr; i++)
+		feed_object(&negotiated->oid[i], po_in, 1);
 
 	while (refs) {
 		if (!is_null_oid(&refs->old_oid))
@@ -409,11 +413,55 @@ static void reject_invalid_nonce(const char *nonce, int len)
 	}
 }
 
+static void get_commons_through_negotiation(const char *url,
+					    const struct ref *remote_refs,
+					    struct oid_array *commons)
+{
+	struct child_process child = CHILD_PROCESS_INIT;
+	const struct ref *ref;
+	int len = the_hash_algo->hexsz + 1; /* hash + NL */
+
+	child.git_cmd = 1;
+	child.no_stdin = 1;
+	child.out = -1;
+	strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
+	for (ref = remote_refs; ref; ref = ref->next)
+		strvec_pushf(&child.args, "--negotiation-tip=%s", oid_to_hex(&ref->new_oid));
+	strvec_push(&child.args, url);
+
+	if (start_command(&child))
+		die(_("send-pack: unable to fork off fetch subprocess"));
+
+	do {
+		char hex_hash[GIT_MAX_HEXSZ + 1];
+		int read_len = read_in_full(child.out, hex_hash, len);
+		struct object_id oid;
+		const char *end;
+
+		if (!read_len)
+			break;
+		if (read_len != len)
+			die("invalid length read %d", read_len);
+		if (parse_oid_hex(hex_hash, &oid, &end) || *end != '\n')
+			die("invalid hash");
+		oid_array_append(commons, &oid);
+	} while (1);
+
+	if (finish_command(&child)) {
+		/*
+		 * The information that push negotiation provides is useful but
+		 * not mandatory.
+		 */
+		warning(_("push negotiation failed; proceeding anyway with push"));
+	}
+}
+
 int send_pack(struct send_pack_args *args,
 	      int fd[], struct child_process *conn,
 	      struct ref *remote_refs,
 	      struct oid_array *extra_have)
 {
+	struct oid_array commons = OID_ARRAY_INIT;
 	int in = fd[0];
 	int out = fd[1];
 	struct strbuf req_buf = STRBUF_INIT;
@@ -426,6 +474,7 @@ int send_pack(struct send_pack_args *args,
 	int quiet_supported = 0;
 	int agent_supported = 0;
 	int advertise_sid = 0;
+	int push_negotiate = 0;
 	int use_atomic = 0;
 	int atomic_supported = 0;
 	int use_push_options = 0;
@@ -437,6 +486,10 @@ int send_pack(struct send_pack_args *args,
 	const char *push_cert_nonce = NULL;
 	struct packet_reader reader;
 
+	git_config_get_bool("push.negotiate", &push_negotiate);
+	if (push_negotiate)
+		get_commons_through_negotiation(args->url, remote_refs, &commons);
+
 	git_config_get_bool("transfer.advertisesid", &advertise_sid);
 
 	/* Does the other end support the reporting? */
@@ -625,7 +678,7 @@ int send_pack(struct send_pack_args *args,
 			   PACKET_READ_DIE_ON_ERR_PACKET);
 
 	if (need_pack_data && cmds_sent) {
-		if (pack_objects(out, remote_refs, extra_have, args) < 0) {
+		if (pack_objects(out, remote_refs, extra_have, &commons, args) < 0) {
 			if (args->stateless_rpc)
 				close(out);
 			if (git_connection_is_socket(conn))
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index f11742ed59..0916f76302 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -191,6 +191,41 @@ test_expect_success 'fetch with pushInsteadOf (should not rewrite)' '
 	)
 '
 
+grep_wrote () {
+	object_count=$1
+	file_name=$2
+	grep 'write_pack_file/wrote.*"value":"'$1'"' $2
+}
+
+test_expect_success 'push with negotiation' '
+	# Without negotiation
+	mk_empty testrepo &&
+	git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
+	git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
+	echo now pushing without negotiation &&
+	GIT_TRACE2_EVENT="$(pwd)/event" git -c protocol.version=2 push testrepo refs/heads/main:refs/remotes/origin/main &&
+	grep_wrote 5 event && # 2 commits, 2 trees, 1 blob
+
+	# Same commands, but with negotiation
+	rm event &&
+	mk_empty testrepo &&
+	git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
+	git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
+	GIT_TRACE2_EVENT="$(pwd)/event" git -c protocol.version=2 -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main &&
+	grep_wrote 2 event # 1 commit, 1 tree
+'
+
+test_expect_success 'push with negotiation proceeds anyway even if negotiation fails' '
+	rm event &&
+	mk_empty testrepo &&
+	git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
+	git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
+	GIT_TEST_PROTOCOL_VERSION=0 GIT_TRACE2_EVENT="$(pwd)/event" \
+		git -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main 2>err &&
+	grep_wrote 5 event && # 2 commits, 2 trees, 1 blob
+	test_i18ngrep "push negotiation failed" err
+'
+
 test_expect_success 'push without wildcard' '
 	mk_empty testrepo &&
 
-- 
2.31.1.527.g47e6f16901-goog


      parent reply	other threads:[~2021-05-04 21:16 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-09  1:09 [PATCH 0/6] Push negotiation Jonathan Tan
2021-04-09  1:09 ` [PATCH 1/6] fetch-pack: buffer object-format with other args Jonathan Tan
2021-04-09  4:49   ` Junio C Hamano
2021-04-09 16:24     ` Jonathan Tan
2021-04-09  1:09 ` [PATCH 2/6] fetch-pack: refactor process_acks() Jonathan Tan
2021-04-09  5:08   ` Junio C Hamano
2021-05-03 16:30   ` Derrick Stolee
2021-04-09  1:10 ` [PATCH 3/6] fetch-pack: refactor add_haves() Jonathan Tan
2021-04-09  5:20   ` Junio C Hamano
2021-04-09  1:10 ` [PATCH 4/6] fetch-pack: refactor command and capability write Jonathan Tan
2021-04-09  5:27   ` Junio C Hamano
2021-04-09  1:10 ` [PATCH 5/6] fetch: teach independent negotiation (no packfile) Jonathan Tan
2021-04-09  5:41   ` Junio C Hamano
2021-04-09 16:38     ` Jonathan Tan
2021-05-03 15:25   ` Derrick Stolee
2021-05-03 15:40     ` Derrick Stolee
2021-05-03 21:52     ` Jonathan Tan
2021-04-09  1:10 ` [PATCH 6/6] send-pack: support push negotiation Jonathan Tan
2021-05-03 15:35   ` Derrick Stolee
2021-05-03 22:02     ` Jonathan Tan
2021-05-04 17:26       ` Derrick Stolee
2021-04-30  5:42 ` [PATCH 0/6] Push negotiation Junio C Hamano
2021-04-30 17:33   ` Derrick Stolee
2021-05-04 21:15 ` [PATCH v2 0/5] " Jonathan Tan
2021-05-04 21:15   ` [PATCH v2 1/5] fetch-pack: refactor process_acks() Jonathan Tan
2021-05-04 21:15   ` [PATCH v2 2/5] fetch-pack: refactor add_haves() Jonathan Tan
2021-05-04 21:16   ` [PATCH v2 3/5] fetch-pack: refactor command and capability write Jonathan Tan
2021-05-04 21:16   ` [PATCH v2 4/5] fetch: teach independent negotiation (no packfile) Jonathan Tan
2021-05-05  1:53     ` Junio C Hamano
2021-05-05 16:42       ` Derrick Stolee
2021-05-06  2:12         ` Junio C Hamano
2021-05-05 16:44     ` Derrick Stolee
2021-05-04 21:16   ` Jonathan Tan [this message]

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=d38a8b7d66efee74e327c4a7f4322e2c02a7495d.1620162764.git.jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=stolee@gmail.com \
    --subject='Re: [PATCH v2 5/5] send-pack: support push negotiation' \
    /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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

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

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git