diff --git a/0001-fetch-pack-return-enum-from-process_acks.patch b/0001-fetch-pack-return-enum-from-process_acks.patch new file mode 100644 index 0000000..b713995 --- /dev/null +++ b/0001-fetch-pack-return-enum-from-process_acks.patch @@ -0,0 +1,85 @@ +From 2531a904f87afcf813757bcaad303a463812f6cb Mon Sep 17 00:00:00 2001 +From: Jonathan Tan +Date: Mon, 27 Apr 2020 17:01:08 -0700 +Subject: [PATCH] fetch-pack: return enum from process_acks() + +References: bsc#1170741 +Upstream: queued - expected 2.26.3 + +process_acks() returns 0, 1, or 2, depending on whether "ready" was +received and if not, whether at least one commit was found to be common. +Replace these magic numbers with a documented enum. + +Signed-off-by: Jonathan Tan +Signed-off-by: Junio C Hamano +--- + fetch-pack.c | 35 ++++++++++++++++++++++++++++------- + 1 file changed, 28 insertions(+), 7 deletions(-) + +diff --git a/fetch-pack.c b/fetch-pack.c +index 1734a573b0..ffdec5e56b 100644 +--- a/fetch-pack.c ++++ b/fetch-pack.c +@@ -1268,9 +1268,29 @@ static int process_section_header(struct packet_reader *reader, + return ret; + } + +-static int process_acks(struct fetch_negotiator *negotiator, +- struct packet_reader *reader, +- struct oidset *common) ++enum common_found { ++ /* ++ * No commit was found to be possessed by both the client and the ++ * server, and "ready" was not received. ++ */ ++ NO_COMMON_FOUND, ++ ++ /* ++ * At least one commit was found to be possessed by both the client and ++ * the server, and "ready" was not received. ++ */ ++ COMMON_FOUND, ++ ++ /* ++ * "ready" was received, indicating that the server is ready to send ++ * the packfile without any further negotiation. ++ */ ++ READY ++}; ++ ++static enum common_found process_acks(struct fetch_negotiator *negotiator, ++ struct packet_reader *reader, ++ struct oidset *common) + { + /* received */ + int received_ready = 0; +@@ -1320,7 +1340,8 @@ static int process_acks(struct fetch_negotiator *negotiator, + die(_("expected no other sections to be sent after no 'ready'")); + + /* return 0 if no common, 1 if there are common, or 2 if ready */ +- return received_ready ? 2 : (received_ack ? 1 : 0); ++ return received_ready ? READY : ++ (received_ack ? COMMON_FOUND : NO_COMMON_FOUND); + } + + static void receive_shallow_info(struct fetch_pack_args *args, +@@ -1508,13 +1529,13 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, + case FETCH_PROCESS_ACKS: + /* Process ACKs/NAKs */ + switch (process_acks(negotiator, &reader, &common)) { +- case 2: ++ case READY: + state = FETCH_GET_PACK; + break; +- case 1: ++ case COMMON_FOUND: + in_vain = 0; + /* fallthrough */ +- default: ++ case NO_COMMON_FOUND: + state = FETCH_SEND_REQUEST; + break; + } +-- +2.26.1 + diff --git a/0002-fetch-pack-in-protocol-v2-in_vain-only-after-ACK.patch b/0002-fetch-pack-in-protocol-v2-in_vain-only-after-ACK.patch new file mode 100644 index 0000000..365c093 --- /dev/null +++ b/0002-fetch-pack-in-protocol-v2-in_vain-only-after-ACK.patch @@ -0,0 +1,128 @@ +From cbfc00c0a924a625f29a8f696d1592785d0ed953 Mon Sep 17 00:00:00 2001 +From: Jonathan Tan +Date: Mon, 27 Apr 2020 17:01:09 -0700 +Subject: [PATCH] fetch-pack: in protocol v2, in_vain only after ACK + +References: bsc#1170741 +Upstream: queued - expected 2.26.3 + +When fetching, Git stops negotiation when it has sent at least +MAX_IN_VAIN (which is 256) "have" lines without having any of them +ACK-ed. But this is supposed to trigger only after the first ACK, as +pack-protocol.txt says: + + However, the 256 limit *only* turns on in the canonical client + implementation if we have received at least one "ACK %s continue" + during a prior round. This helps to ensure that at least one common + ancestor is found before we give up entirely. + +The code path for protocol v0 observes this, but not protocol v2, +resulting in shorter negotiation rounds but significantly larger +packfiles. Teach the code path for protocol v2 to check this criterion +only after at least one ACK was received. + +Signed-off-by: Jonathan Tan +Signed-off-by: Junio C Hamano +--- + fetch-pack.c | 13 +++++++++---- + t/t5500-fetch-pack.sh | 18 ++++++++++++++++++ + 2 files changed, 27 insertions(+), 4 deletions(-) + +diff --git a/fetch-pack.c b/fetch-pack.c +index ffdec5e56b..8f50f6a248 100644 +--- a/fetch-pack.c ++++ b/fetch-pack.c +@@ -1143,6 +1143,7 @@ static void add_common(struct strbuf *req_buf, struct oidset *common) + } + + static int add_haves(struct fetch_negotiator *negotiator, ++ int seen_ack, + struct strbuf *req_buf, + int *haves_to_send, int *in_vain) + { +@@ -1157,7 +1158,7 @@ static int add_haves(struct fetch_negotiator *negotiator, + } + + *in_vain += haves_added; +- if (!haves_added || *in_vain >= MAX_IN_VAIN) { ++ if (!haves_added || (seen_ack && *in_vain >= MAX_IN_VAIN)) { + /* Send Done */ + packet_buf_write(req_buf, "done\n"); + ret = 1; +@@ -1173,7 +1174,7 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out, + struct fetch_pack_args *args, + const struct ref *wants, struct oidset *common, + int *haves_to_send, int *in_vain, +- int sideband_all) ++ int sideband_all, int seen_ack) + { + int ret = 0; + struct strbuf req_buf = STRBUF_INIT; +@@ -1230,7 +1231,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out, + add_common(&req_buf, common); + + /* Add initial haves */ +- ret = add_haves(negotiator, &req_buf, haves_to_send, in_vain); ++ ret = add_haves(negotiator, seen_ack, &req_buf, ++ haves_to_send, in_vain); + } + + /* Send request */ +@@ -1465,6 +1467,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, + int haves_to_send = INITIAL_FLUSH; + struct fetch_negotiator negotiator_alloc; + struct fetch_negotiator *negotiator; ++ int seen_ack = 0; + + if (args->no_dependents) { + negotiator = NULL; +@@ -1521,7 +1524,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, + if (send_fetch_request(negotiator, fd[1], args, ref, + &common, + &haves_to_send, &in_vain, +- reader.use_sideband)) ++ reader.use_sideband, ++ seen_ack)) + state = FETCH_GET_PACK; + else + state = FETCH_PROCESS_ACKS; +@@ -1534,6 +1538,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, + break; + case COMMON_FOUND: + in_vain = 0; ++ seen_ack = 1; + /* fallthrough */ + case NO_COMMON_FOUND: + state = FETCH_SEND_REQUEST; +diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh +index 6b97923964..95ed08db1b 100755 +--- a/t/t5500-fetch-pack.sh ++++ b/t/t5500-fetch-pack.sh +@@ -385,6 +385,24 @@ test_expect_success 'clone shallow with packed refs' ' + test_cmp count8.expected count8.actual + ' + ++test_expect_success 'in_vain not triggered before first ACK' ' ++ rm -rf myserver myclient trace && ++ git init myserver && ++ test_commit -C myserver foo && ++ git clone "file://$(pwd)/myserver" myclient && ++ ++ # MAX_IN_VAIN is 256. Because of batching, the client will send 496 ++ # (16+32+64+128+256) commits, not 256, before giving up. So create 496 ++ # irrelevant commits. ++ test_commit_bulk -C myclient 496 && ++ ++ # The new commit that the client wants to fetch. ++ test_commit -C myserver bar && ++ ++ GIT_TRACE_PACKET="$(pwd)/trace" git -C myclient fetch --progress origin && ++ test_i18ngrep "Total 3 " trace ++' ++ + test_expect_success 'fetch in shallow repo unreachable shallow objects' ' + ( + git clone --bare --branch B --single-branch "file://$(pwd)/." no-reflog && +-- +2.26.1 + diff --git a/0003-fetch-pack-in-protocol-v2-reset-in_vain-upon-ACK.patch b/0003-fetch-pack-in-protocol-v2-reset-in_vain-upon-ACK.patch new file mode 100644 index 0000000..be96a1f --- /dev/null +++ b/0003-fetch-pack-in-protocol-v2-reset-in_vain-upon-ACK.patch @@ -0,0 +1,80 @@ +From 30eb7668fb1108c9d509bccf2bc5d53d71b3f435 Mon Sep 17 00:00:00 2001 +From: Jonathan Tan +Date: Mon, 27 Apr 2020 17:01:10 -0700 +Subject: [PATCH] fetch-pack: in protocol v2, reset in_vain upon ACK + +References: bsc#1170741 +Upstream: queued - expected 2.26.3 + +In the function process_acks() in fetch-pack.c, the variable +received_ack is meant to track that an ACK was received, but it was +never set. This results in negotiation terminating prematurely through +the in_vain counter, when the counter should have been reset upon every +ACK. + +Therefore, reset the in_vain counter upon every ACK. + +Helped-by: Jonathan Nieder +Signed-off-by: Jonathan Tan +Signed-off-by: Junio C Hamano +--- + fetch-pack.c | 1 + + t/t5500-fetch-pack.sh | 30 ++++++++++++++++++++++++++++++ + 2 files changed, 31 insertions(+) + +diff --git a/fetch-pack.c b/fetch-pack.c +index 8f50f6a248..4b48090217 100644 +--- a/fetch-pack.c ++++ b/fetch-pack.c +@@ -1307,6 +1307,7 @@ static enum common_found process_acks(struct fetch_negotiator *negotiator, + + if (skip_prefix(reader->line, "ACK ", &arg)) { + struct object_id oid; ++ received_ack = 1; + if (!get_oid_hex(arg, &oid)) { + struct commit *commit; + oidset_insert(common, &oid); +diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh +index 95ed08db1b..fd39aad93f 100755 +--- a/t/t5500-fetch-pack.sh ++++ b/t/t5500-fetch-pack.sh +@@ -403,6 +403,36 @@ test_expect_success 'in_vain not triggered before first ACK' ' + test_i18ngrep "Total 3 " trace + ' + ++test_expect_success 'in_vain resetted upon ACK' ' ++ rm -rf myserver myclient trace && ++ git init myserver && ++ ++ # Linked list of commits on master. The first is common; the rest are ++ # not. ++ test_commit -C myserver first_master_commit && ++ git clone "file://$(pwd)/myserver" myclient && ++ test_commit_bulk -C myclient 255 && ++ ++ # Another linked list of commits on anotherbranch with no connection to ++ # master. The first is common; the rest are not. ++ git -C myserver checkout --orphan anotherbranch && ++ test_commit -C myserver first_anotherbranch_commit && ++ git -C myclient fetch origin anotherbranch:refs/heads/anotherbranch && ++ git -C myclient checkout anotherbranch && ++ test_commit_bulk -C myclient 255 && ++ ++ # The new commit that the client wants to fetch. ++ git -C myserver checkout master && ++ test_commit -C myserver to_fetch && ++ ++ # The client will send (as "have"s) all 256 commits in anotherbranch ++ # first. The 256th commit is common between the client and the server, ++ # and should reset in_vain. This allows negotiation to continue until ++ # the client reports that first_anotherbranch_commit is common. ++ GIT_TRACE_PACKET="$(pwd)/trace" git -C myclient fetch --progress origin master && ++ test_i18ngrep "Total 3 " trace ++' ++ + test_expect_success 'fetch in shallow repo unreachable shallow objects' ' + ( + git clone --bare --branch B --single-branch "file://$(pwd)/." no-reflog && +-- +2.26.1 + diff --git a/Revert-fetch-default-to-protocol-version-2.patch b/Revert-fetch-default-to-protocol-version-2.patch deleted file mode 100644 index b9faca5..0000000 --- a/Revert-fetch-default-to-protocol-version-2.patch +++ /dev/null @@ -1,42 +0,0 @@ -From 3063fdec2a274235925c1b202ba8c65537e48211 Mon Sep 17 00:00:00 2001 -From: Michal Suchanek -Date: Tue, 21 Apr 2020 21:38:48 +0200 -Subject: [PATCH] Revert "fetch: default to protocol version 2" - -This reverts commit 684ceae32dae726c6a5c693b257b156926aba8b7. - -Causes a lot of data fetched from the Linux tree. ---- - Documentation/config/protocol.txt | 2 +- - protocol.c | 2 +- - 2 files changed, 2 insertions(+), 2 deletions(-) - -diff --git a/Documentation/config/protocol.txt b/Documentation/config/protocol.txt -index 756591d77b08..0b40141613e3 100644 ---- a/Documentation/config/protocol.txt -+++ b/Documentation/config/protocol.txt -@@ -48,7 +48,7 @@ protocol.version:: - If set, clients will attempt to communicate with a server - using the specified protocol version. If the server does - not support it, communication falls back to version 0. -- If unset, the default is `2`. -+ If unset, the default is `0`. - Supported versions: - + - -- -diff --git a/protocol.c b/protocol.c -index 803bef5c87e0..d390391ebac8 100644 ---- a/protocol.c -+++ b/protocol.c -@@ -39,7 +39,7 @@ enum protocol_version get_protocol_version_config(void) - return env; - } - -- return protocol_v2; -+ return protocol_v0; - } - - enum protocol_version determine_protocol_version_server(void) --- -2.26.0 - diff --git a/git.changes b/git.changes index 0b47ed4..5c6f5f6 100644 --- a/git.changes +++ b/git.changes @@ -1,10 +1,24 @@ +------------------------------------------------------------------- +Tue Apr 28 17:42:18 UTC 2020 - Michal Suchanek + +- Protocol v2 in_vain fixes (bsc#1170741). + Dropped: Revert-fetch-default-to-protocol-version-2.patch + Added: 0001-fetch-pack-return-enum-from-process_acks.patch + 0002-fetch-pack-in-protocol-v2-in_vain-only-after-ACK.patch + 0003-fetch-pack-in-protocol-v2-reset-in_vain-upon-ACK.patch + +------------------------------------------------------------------- +Mon Apr 27 13:36:27 UTC 2020 - Michal Suchanek + +- Add back SuSEfirewall2 support needed for SLE12 (bsc#1170302). + ------------------------------------------------------------------- Tue Apr 21 19:41:46 UTC 2020 - Michal Suchanek - With recent switch to protocol v2 people are reporting fetches transferring unreasonable amount of data. Upstream proposes switching the protocol back until the issue is properly diagnosed. The regression is problematic for - people with lower network connection speed. + people with lower network connection speed (bsc#1170741). Added: Revert-fetch-default-to-protocol-version-2.patch ------------------------------------------------------------------- diff --git a/git.spec b/git.spec index 7a0f968..1b3c9b6 100644 --- a/git.spec +++ b/git.spec @@ -17,6 +17,10 @@ %define gitexecdir %{_libexecdir}/git +%if 0%{?suse_version} < 1500 +%define _fwdefdir %{_sysconfdir}/sysconfig/SuSEfirewall2.d/services +%define SuSEfirewall2 1 +%endif #Compat macro for new _fillupdir macro introduced in Nov 2017 %if ! %{defined _fillupdir} @@ -43,6 +47,7 @@ Source1: apache2-gitweb.conf Source2: sysconfig.git-daemon Source3: git-daemon.service Source5: usr.share.git-web.gitweb.cgi +Source6: susefirewall-git-daemon Source7: https://www.kernel.org/pub/software/scm/git/%{name}-%{version}.tar.sign Source8: %{name}.keyring Source9: %{name}-gui.desktop @@ -58,7 +63,10 @@ Patch8: git-asciidoc.patch Patch10: setup-don-t-fail-if-commondir-reference-is-deleted.patch Patch11: 0001-DOC-Move-to-DocBook-5-when-using-asciidoctor.patch Patch13: 0002-Also-use-DocBook-5-stylesheet-when-generating-HTML-o.patch -Patch14: Revert-fetch-default-to-protocol-version-2.patch +Patch14: 0001-fetch-pack-return-enum-from-process_acks.patch +Patch15: 0002-fetch-pack-in-protocol-v2-in_vain-only-after-ACK.patch +Patch16: 0003-fetch-pack-in-protocol-v2-reset-in_vain-upon-ACK.patch + BuildRequires: fdupes BuildRequires: gpg2 BuildRequires: libcurl-devel @@ -288,6 +296,8 @@ directory /git/ that calls the cgi script. %patch11 -p1 %patch13 -p1 %patch14 -p1 +%patch15 -p1 +%patch16 -p1 %build cat > .make <<'EOF' @@ -346,6 +356,10 @@ ln -s %{_sbindir}/service %{buildroot}%{_sbindir}/rcgit-daemon install -d -m 755 %{buildroot}%{_fillupdir} install -m 644 %{SOURCE2} %{buildroot}%{_fillupdir}/sysconfig.git-daemon install -d -m 755 %{buildroot}/srv/git +%if 0%{?SuSEfirewall2} +mkdir -p %{buildroot}/%{_fwdefdir} +install -m 644 %{SOURCE6} %{buildroot}/%{_fwdefdir}/git-daemon +%endif ### ./.make -C contrib/subtree install %{!?_without_docs: ./.make -C contrib/subtree install-doc} @@ -476,6 +490,9 @@ fi %dir /srv/git %{_fillupdir}/sysconfig.git-daemon %{!?_without_docs: %{_mandir}/man1/git-daemon.1*} +%if 0%{?SuSEfirewall2} +%config %{_fwdefdir}/* +%endif %files -n gitk %{_bindir}/gitk diff --git a/susefirewall-git-daemon b/susefirewall-git-daemon new file mode 100644 index 0000000..493d71a --- /dev/null +++ b/susefirewall-git-daemon @@ -0,0 +1,4 @@ +## Name: git-daemon +## Description: Open ports for git-daemon + +TCP="git"