[vlc-devel] [PATCH] contrib: gnutls: fix 3.6 compilation with an older macOS SDK

[Steve Lhomme]

On 2020-06-20 21:41, David Fuhrmann wrote:
> 
> 
>> Am 20.06.2020 um 09:15 schrieb Steve Lhomme <robux4 at ycbcr.xyz>:
>>
>> Fixes this compilation error:
>> system/fastopen.c:134:9: error: 'connectx' is only available on macOS 10.11 or newer [-Werror,-Wunguarded-availability]
>>                 ret = connectx(fd, &endpoints, SAE_ASSOCID_ANY, CONNECT_RESUME_ON_READ_WRITE | CONNECT_DATA_IDEMPOTENT, NULL, 0, NULL, NULL);
>>                       ^~~~~~~~
>> /Applications/Xcode9.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/usr/include/sys/socket.h:713:5:
>> note: 'connectx' has been marked as being introduced in macOS 10.11 here, but the deployment target is macOS 10.7.0
>>
>> In this patch __builtin_available() is assumed to be avalaible in the toolchain
>> which is the case in our 3.0 and 4.0 toolchains. A cleaner patch should detect
>> it in configure.ac. But we can't do autoconf changes in gnutls as it never
>> works properly.
>> ---
>> ...ctx-not-available-on-older-macOS-SDK.patch | 46 +++++++++++++++++++
>> contrib/src/gnutls/rules.mak                  |  3 ++
>> 2 files changed, 49 insertions(+)
>> create mode 100644 contrib/src/gnutls/0001-fix-connectx-not-available-on-older-macOS-SDK.patch
>>
>> diff --git a/contrib/src/gnutls/0001-fix-connectx-not-available-on-older-macOS-SDK.patch b/contrib/src/gnutls/0001-fix-connectx-not-available-on-older-macOS-SDK.patch
>> new file mode 100644
>> index 00000000000..300437a2320
>> --- /dev/null
>> +++ b/contrib/src/gnutls/0001-fix-connectx-not-available-on-older-macOS-SDK.patch
>> @@ -0,0 +1,46 @@
>> +From 3faffe77fd5cb15cb132ebaf4bfef6dc579f25e2 Mon Sep 17 00:00:00 2001
>> +From: Steve Lhomme <robux4 at ycbcr.xyz>
>> +Date: Fri, 19 Jun 2020 15:11:00 +0200
>> +Subject: [PATCH] fix connectx not available on older macOS SDK
>> +
>> +Fixes this compilation error:
>> +system/fastopen.c:134:9: error: 'connectx' is only available on macOS 10.11 or newer [-Werror,-Wunguarded-availability]
>> +                ret = connectx(fd, &endpoints, SAE_ASSOCID_ANY, CONNECT_RESUME_ON_READ_WRITE | CONNECT_DATA_IDEMPOTENT, NULL, 0, NULL, NULL);
>> +                      ^~~~~~~~
>> +/Applications/Xcode9.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/usr/include/sys/socket.h:713:5: note: 'connectx' has been marked as being introduced in macOS 10.11 here, but the deployment target is macOS 10.7.0
>> +
>> +The detection is the same as found in curl [1].
>> +
>> +If HAVE_BUILTIN_AVAILABLE is not available we fallback to the code without
>> +TCP_FASTOPEN_OSX.
>> +
>> +[1] https://github.com/curl/curl/commit/870d849d48a26b8eeb0d4bb1f4655367a4a191ca
>> +---
>> + lib/system/fastopen.c | 10 ++++++++--
>> + 1 file changed, 8 insertions(+), 2 deletions(-)
>> +
>> +diff --git a/lib/system/fastopen.c b/lib/system/fastopen.c
>> +index 8d8409e48..b816decff 100644
>> +--- a/lib/system/fastopen.c
>> ++++ b/lib/system/fastopen.c
>> +@@ -129,9 +129,15 @@ tfo_writev(gnutls_transport_ptr_t ptr, const giovec_t * iovec, int iovec_cnt)
>> + 	}
>> + # elif defined(TCP_FASTOPEN_OSX)
>> + 	{
>> +-		sa_endpoints_t endpoints = { .sae_dstaddr = (struct sockaddr*)&p->connect_addr, .sae_dstaddrlen = p->connect_addrlen };
>> ++		if(__builtin_available(macOS 10.11, iOS 9.0, tvOS 9.0, watchOS 2.0, *)) {
>> ++			sa_endpoints_t endpoints = { .sae_dstaddr = (struct sockaddr*)&p->connect_addr, .sae_dstaddrlen = p->connect_addrlen };
>> +
>> +-		ret = connectx(fd, &endpoints, SAE_ASSOCID_ANY, CONNECT_RESUME_ON_READ_WRITE | CONNECT_DATA_IDEMPOTENT, NULL, 0, NULL, NULL);
>> ++			ret = connectx(fd, &endpoints, SAE_ASSOCID_ANY, CONNECT_RESUME_ON_READ_WRITE | CONNECT_DATA_IDEMPOTENT, NULL, 0, NULL, NULL);
>> ++		}
>> ++		else
>> ++		{
>> ++			ret = connect(fd, (struct sockaddr*)&p->connect_addr, p->connect_addrlen);
>> ++		}
>> + 		if (errno == ENOTCONN || errno == EINPROGRESS) {
>> + 			gnutls_assert();
>> + 			errno = EAGAIN;
>> +--
>> +2.26.0.windows.1
>> +
>> diff --git a/contrib/src/gnutls/rules.mak b/contrib/src/gnutls/rules.mak
>> index 191dd8040ca..96a5613a660 100644
>> --- a/contrib/src/gnutls/rules.mak
>> +++ b/contrib/src/gnutls/rules.mak
>> @@ -33,6 +33,9 @@ gnutls: gnutls-$(GNUTLS_VERSION).tar.xz .sum-gnutls
>>
>> 	# disable the dllimport in static linking (pkg-config --static doesn't handle Cflags.private)
>> 	cd $(UNPACK_DIR) && sed -i.orig -e s/"_SYM_EXPORT __declspec(dllimport)"/"_SYM_EXPORT"/g lib/includes/gnutls/gnutls.h.in
>> +
>> +	# don't use connectx on macOS versions where it's not available
>> +	$(APPLY) $(SRC)/gnutls/0001-fix-connectx-not-available-on-older-macOS-SDK.patch
>> ifdef HAVE_ANDROID
>> 	$(APPLY) $(SRC)/gnutls/no-create-time-h.patch
>> endif
> 
> LGTM, but this is only needed if you intend to also port a gnutls bump to 3.6 to the VLC-3.0 branch, and the patch itself is only needed for vlc-3.0 as well, as 4.0 will have 10.11 as macOS min version.

Yes it's only needed in 3.0. Also as a general rule it's better if 
patches apply independently of the compiled target (no HAVE_WINSTORE), 
if it's an upstreamed patch and if 3.0 and 4.0 create the same contrib 
from the same version. So I would rather keep them in sync.

I hope to have a variant of this patch upstreamed so we don't have much 
to patch anymore.