git: Backport patch to fix with curl 8.7.0 onwards

bitbake-selftest was failing on a github url on hosts using buildtools.
The issue was tracked down to the curl upgrade 8.6.0 -> 8.7.1 and there
is a fix in upstream git we can backport to address it.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 ...98a77463e9d24c19c8d9473fd2152d5840c4.patch | 195 ++++++++++++++++++
 meta/recipes-devtools/git/git_2.44.0.bb       |   1 +
 2 files changed, 196 insertions(+)
 create mode 100644 meta/recipes-devtools/git/git/eba498a77463e9d24c19c8d9473fd2152d5840c4.patch

On Thu, Apr 18, 2024 at 1:05 AM Richard Purdie via
lists.openembedded.org
<richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote:
>
> bitbake-selftest was failing on a github url on hosts using buildtools.
> The issue was tracked down to the curl upgrade 8.6.0 -> 8.7.1 and there
> is a fix in upstream git we can backport to address it.
>

Thanks for tracking it down. LGTM

> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  ...98a77463e9d24c19c8d9473fd2152d5840c4.patch | 195 ++++++++++++++++++
>  meta/recipes-devtools/git/git_2.44.0.bb       |   1 +
>  2 files changed, 196 insertions(+)
>  create mode 100644 meta/recipes-devtools/git/git/eba498a77463e9d24c19c8d9473fd2152d5840c4.patch
>
> diff --git a/meta/recipes-devtools/git/git/eba498a77463e9d24c19c8d9473fd2152d5840c4.patch b/meta/recipes-devtools/git/git/eba498a77463e9d24c19c8d9473fd2152d5840c4.patch
> new file mode 100644
> index 00000000000..cd04cef30df
> --- /dev/null
> +++ b/meta/recipes-devtools/git/git/eba498a77463e9d24c19c8d9473fd2152d5840c4.patch
> @@ -0,0 +1,195 @@
> +Backport a fix from upstream for curl 8.7 compatibility.
> +
> +Upstream-Status: Backport
> +
> +From 324231174247c70e66908b78924908fbfcebed19 Mon Sep 17 00:00:00 2001
> +From: Jeff King <peff@peff.net>
> +Date: Tue, 2 Apr 2024 16:05:17 -0400
> +Subject: [PATCH 1/3] http: reset POSTFIELDSIZE when clearing curl handle
> +
> +In get_active_slot(), we return a CURL handle that may have been used
> +before (reusing them is good because it lets curl reuse the same
> +connection across many requests). We set a few curl options back to
> +defaults that may have been modified by previous requests.
> +
> +We reset POSTFIELDS to NULL, but do not reset POSTFIELDSIZE (which
> +defaults to "-1"). This usually doesn't matter because most POSTs will
> +set both fields together anyway. But there is one exception: when
> +handling a large request in remote-curl's post_rpc(), we don't set
> +_either_, and instead set a READFUNCTION to stream data into libcurl.
> +
> +This can interact weirdly with a stale POSTFIELDSIZE setting, because
> +curl will assume it should read only some set number of bytes from our
> +READFUNCTION. However, it has worked in practice because we also
> +manually set a "Transfer-Encoding: chunked" header, which libcurl uses
> +as a clue to set the POSTFIELDSIZE to -1 itself.
> +
> +So everything works, but we're better off resetting the size manually
> +for a few reasons:
> +
> +  - there was a regression in curl 8.7.0 where the chunked header
> +    detection didn't kick in, causing any large HTTP requests made by
> +    Git to fail. This has since been fixed (but not yet released). In
> +    the issue, curl folks recommended setting it explicitly to -1:
> +
> +      https://github.com/curl/curl/issues/13229#issuecomment-2029826058
> +
> +    and it indeed works around the regression. So even though it won't
> +    be strictly necessary after the fix there, this will help folks who
> +    end up using the affected libcurl versions.
> +
> +  - it's consistent with what a new curl handle would look like. Since
> +    get_active_slot() may or may not return a used handle, this reduces
> +    the possibility of heisenbugs that only appear with certain request
> +    patterns.
> +
> +Note that the recommendation in the curl issue is to actually drop the
> +manual Transfer-Encoding header. Modern libcurl will add the header
> +itself when streaming from a READFUNCTION. However, that code wasn't
> +added until 802aa5ae2 (HTTP: use chunked Transfer-Encoding for HTTP_POST
> +if size unknown, 2019-07-22), which is in curl 7.66.0. We claim to
> +support back to 7.19.5, so those older versions still need the manual
> +header.
> +
> +Signed-off-by: Jeff King <peff@peff.net>
> +Signed-off-by: Junio C Hamano <gitster@pobox.com>
> +---
> + http.c | 1 +
> + 1 file changed, 1 insertion(+)
> +
> +diff --git a/http.c b/http.c
> +index e73b136e5897bd..3d80bd6116e9e4 100644
> +--- a/http.c
> ++++ b/http.c
> +@@ -1452,6 +1452,7 @@ struct active_request_slot *get_active_slot(void)
> +       curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, NULL);
> +       curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, NULL);
> +       curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, NULL);
> ++      curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, -1L);
> +       curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);
> +       curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
> +       curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);
> +
> +From c28ee09503ffcb1f32cbb30dc57fa810cdac122c Mon Sep 17 00:00:00 2001
> +From: Jeff King <peff@peff.net>
> +Date: Tue, 2 Apr 2024 16:06:00 -0400
> +Subject: [PATCH 2/3] INSTALL: bump libcurl version to 7.21.3
> +
> +Our documentation claims we support curl versions back to 7.19.5. But we
> +can no longer compile with that version since adding an unconditional
> +use of CURLOPT_RESOLVE in 511cfd3bff (http: add custom hostname to IP
> +address resolutions, 2022-05-16). That feature wasn't added to libcurl
> +until 7.21.3.
> +
> +We could add #ifdefs to make this work back to 7.19.5. But given that
> +nobody noticed the compilation failure in the intervening two years, it
> +makes more sense to bump the version in the documentation to 7.21.3
> +(which is itself over 13 years old).
> +
> +We could perhaps go forward even more (which would let us drop some
> +cruft from git-curl-compat.h), but this should be an obviously safe
> +jump, and we can move forward later.
> +
> +Note that user-visible syntax for CURLOPT_RESOLVE has grown new features
> +in subsequent curl versions. Our documentation mentions "+" and "-"
> +entries, which require more recent versions than 7.21.3. We could
> +perhaps clarify that in our docs, but it's probably not worth cluttering
> +them with restrictions of ancient curl versions.
> +
> +Signed-off-by: Jeff King <peff@peff.net>
> +Signed-off-by: Junio C Hamano <gitster@pobox.com>
> +---
> + INSTALL | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/INSTALL b/INSTALL
> +index c6fb240c91eb90..2a46d045928a11 100644
> +--- a/INSTALL
> ++++ b/INSTALL
> +@@ -139,7 +139,7 @@ Issues of note:
> +         not need that functionality, use NO_CURL to build without
> +         it.
> +
> +-        Git requires version "7.19.5" or later of "libcurl" to build
> ++        Git requires version "7.21.3" or later of "libcurl" to build
> +         without NO_CURL. This version requirement may be bumped in
> +         the future.
> +
> +
> +From 92a209bf245c6497f3742b9df7a1463ddf067ea6 Mon Sep 17 00:00:00 2001
> +From: Jeff King <peff@peff.net>
> +Date: Fri, 5 Apr 2024 16:04:51 -0400
> +Subject: [PATCH 3/3] remote-curl: add Transfer-Encoding header only for older
> + curl
> +
> +As of curl 7.66.0, we don't need to manually specify a "chunked"
> +Transfer-Encoding header. Instead, modern curl deduces the need for it
> +in a POST that has a POSTFIELDSIZE of -1 and uses READFUNCTION rather
> +than POSTFIELDS.
> +
> +That version is recent enough that we can't just drop the header; we
> +need to do so conditionally. Since it's only a single line, it seems
> +like the simplest thing would just be to keep setting it unconditionally
> +(after all, the #ifdefs are much longer than the actual code). But
> +there's another wrinkle: HTTP/2.
> +
> +Curl may choose to use HTTP/2 under the hood if the server supports it.
> +And in that protocol, we do not use the chunked encoding for streaming
> +at all. Most versions of curl handle this just fine by recognizing and
> +removing the header. But there's a regression in curl 8.7.0 and 8.7.1
> +where it doesn't, and large requests over HTTP/2 are broken (which t5559
> +notices). That regression has since been fixed upstream, but not yet
> +released.
> +
> +Make the setting of this header conditional, which will let Git work
> +even with those buggy curl versions. And as a bonus, it serves as a
> +reminder that we can eventually clean up the code as we bump the
> +supported curl versions.
> +
> +Signed-off-by: Jeff King <peff@peff.net>
> +Signed-off-by: Junio C Hamano <gitster@pobox.com>
> +---
> + git-curl-compat.h | 9 +++++++++
> + remote-curl.c     | 3 +++
> + 2 files changed, 12 insertions(+)
> +
> +diff --git a/git-curl-compat.h b/git-curl-compat.h
> +index fd96b3cdffdb6c..e1d0bdd273501f 100644
> +--- a/git-curl-compat.h
> ++++ b/git-curl-compat.h
> +@@ -126,6 +126,15 @@
> + #define GIT_CURL_HAVE_CURLSSLSET_NO_BACKENDS
> + #endif
> +
> ++/**
> ++ * Versions before curl 7.66.0 (September 2019) required manually setting the
> ++ * transfer-encoding for a streaming POST; after that this is handled
> ++ * automatically.
> ++ */
> ++#if LIBCURL_VERSION_NUM < 0x074200
> ++#define GIT_CURL_NEED_TRANSFER_ENCODING_HEADER
> ++#endif
> ++
> + /**
> +  * CURLOPT_PROTOCOLS_STR and CURLOPT_REDIR_PROTOCOLS_STR were added in 7.85.0,
> +  * released in August 2022.
> +diff --git a/remote-curl.c b/remote-curl.c
> +index 1161dc7fed6892..603c11ba667246 100644
> +--- a/remote-curl.c
> ++++ b/remote-curl.c
> +@@ -1,4 +1,5 @@
> + #include "git-compat-util.h"
> ++#include "git-curl-compat.h"
> + #include "config.h"
> + #include "environment.h"
> + #include "gettext.h"
> +@@ -960,7 +961,9 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
> +               /* The request body is large and the size cannot be predicted.
> +                * We must use chunked encoding to send it.
> +                */
> ++#ifdef GIT_CURL_NEED_TRANSFER_ENCODING_HEADER
> +               headers = curl_slist_append(headers, "Transfer-Encoding: chunked");
> ++#endif
> +               rpc->initial_buffer = 1;
> +               curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
> +               curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
> diff --git a/meta/recipes-devtools/git/git_2.44.0.bb b/meta/recipes-devtools/git/git_2.44.0.bb
> index 90e555eba7c..3ae02f29387 100644
> --- a/meta/recipes-devtools/git/git_2.44.0.bb
> +++ b/meta/recipes-devtools/git/git_2.44.0.bb
> @@ -10,6 +10,7 @@ PROVIDES:append:class-native = " git-replacement-native"
>
>  SRC_URI = "${KERNELORG_MIRROR}/software/scm/git/git-${PV}.tar.gz;name=tarball \
>             file://fixsort.patch \
> +           file://eba498a77463e9d24c19c8d9473fd2152d5840c4.patch \
>             file://0001-config.mak.uname-do-not-force-RHEL-7-specific-build-.patch \
>             "
>
> --
> 2.40.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#198504): https://lists.openembedded.org/g/openembedded-core/message/198504
> Mute This Topic: https://lists.openembedded.org/mt/105594103/1997914
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>