Lazy load libcurl, allowing for an SSL/TLS backend-specific libcurl #4410

dscho · 2023-05-07T22:13:09Z

As per #4350 (comment), the major block for upgrading Git for Windows' OpenSSL from v1.1 to v3 is the tricky part where such an upgrade would break git fetch/git clone and git push because the libcurl depends on the OpenSSL DLL, and the major version bump will change the file name of said DLL.

To overcome that, the plan is to build libcurl flavors for each supported SSL/TLS backend, aligning with the way MSYS2 builds libcurl, then switch Git for Windows' SDK to the Secure Channel-flavored libcurl, and teach Git to look for the specific flavor of libcurl corresponding to the http.sslBackend setting (if that was configured).

Here is the PR to teach Git that trick.

Makefile

compat/lazy-load-curl.c

config.mak.uname

dscho · 2023-05-08T12:29:56Z

@rimrul I believe that I have addressed all of your feedback so far.

Range-diff

1: 2d1ad10 ! 1: 55b28f9 http: optionally load libcurl lazily

@@ Makefile: include shared.mak
  #
  #     CURL_LDFLAGS=-lcurl
  #
-+# Define LAZY_LOAD_LIBCURL to dynamically load the libcurl; This can be useful
++# Define LAZYLOAD_LIBCURL to dynamically load the libcurl; This can be useful
 +# if Multiple libcurl versions exist (with different file names) that link to
 +# various SSL/TLS backends, to support the `http.sslBackend` runtime switch in
 +# such a scenario.
@@ Makefile: else
  
 -	ifndef CURL_LDFLAGS
 -		CURL_LDFLAGS = $(eval CURL_LDFLAGS := $$(shell $$(CURL_CONFIG) --libs))$(CURL_LDFLAGS)
-+	ifdef LAZY_LOAD_LIBCURL
-+		LAZY_LOAD_LIBCURL_OBJ = compat/lazy-load-curl.o
-+		OBJECTS += $(LAZY_LOAD_LIBCURL_OBJ)
++	ifdef LAZYLOAD_LIBCURL
++		LAZYLOAD_LIBCURL_OBJ = compat/lazyload-curl.o
++		OBJECTS += $(LAZYLOAD_LIBCURL_OBJ)
++		# The `CURL_STATICLIB` constant must be defined to avoid seeing the functions
++		# declared as DLL imports
 +		CURL_CFLAGS = -DCURL_STATICLIB
 +		CURL_LIBCURL = -ldl
 +	else
@@ Makefile: else
  	ifdef USE_CURL_FOR_IMAP_SEND
  		BASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND
 -		IMAP_SEND_BUILDDEPS = http.o
-+		IMAP_SEND_BUILDDEPS = http.o $(LAZY_LOAD_LIBCURL_OBJ)
++		IMAP_SEND_BUILDDEPS = http.o $(LAZYLOAD_LIBCURL_OBJ)
  		IMAP_SEND_LDFLAGS += $(CURL_LIBCURL)
  	endif
  	ifndef NO_EXPAT
@@ Makefile: git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITL
  		$(IMAP_SEND_LDFLAGS) $(LIBS)
  
 -git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
-+git-http-fetch$X: http.o http-walker.o http-fetch.o $(LAZY_LOAD_LIBCURL_OBJ) GIT-LDFLAGS $(GITLIBS)
++git-http-fetch$X: http.o http-walker.o http-fetch.o $(LAZYLOAD_LIBCURL_OBJ) GIT-LDFLAGS $(GITLIBS)
  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
  		$(CURL_LIBCURL) $(LIBS)
 -git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)
-+git-http-push$X: http.o http-push.o $(LAZY_LOAD_LIBCURL_OBJ) GIT-LDFLAGS $(GITLIBS)
++git-http-push$X: http.o http-push.o $(LAZYLOAD_LIBCURL_OBJ) GIT-LDFLAGS $(GITLIBS)
  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
  		$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
  
@@ Makefile: $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
  	cp $< $@
  
 -$(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS)
-+$(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o $(LAZY_LOAD_LIBCURL_OBJ) GIT-LDFLAGS $(GITLIBS)
++$(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o $(LAZYLOAD_LIBCURL_OBJ) GIT-LDFLAGS $(GITLIBS)
  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
  		$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
  
 
- ## compat/lazy-load-curl.c (new) ##
+ ## compat/lazyload-curl.c (new) ##
 @@
 +#include "../git-compat-util.h"
 +#include "../git-curl-compat.h"
 +#include <dlfcn.h>
 +
++/*
++ * The ABI version of libcurl is encoded in its shared libraries' file names.
++ * This ABI version has not changed since October 2006 and is unlikely to be
++ * changed in the future. See https://curl.se/libcurl/abi.html for details.
++ */
++#define LIBCURL_ABI_VERSION 4
++
 +typedef void (*func_t)(void);
 +
 +#ifdef __APPLE__
-+#define LIBCURL_FILE_NAME(base) base ".4.dylib"
++#define LIBCURL_FILE_NAME(base) base "." #LIBCURL_ABI_VERSION ".dylib"
 +#else
-+#define LIBCURL_FILE_NAME(base) base ".so.4"
++#define LIBCURL_FILE_NAME(base) base ".so." #LIBCURL_ABI_VERSION
 +#endif
 +
 +static void *load_library(const char *name)

2: f1e3572 ! 2: b658d20 http: support lazy-loading libcurl also on Windows

@@ Commit message
     This implements the Windows-specific support code, because everything is
     slightly different on Windows, even loading shared libraries.
 
+    Note: I specifically do _not_ use the code from
+    `compat/win32/lazyload.h` here because that code is optimized for
+    loading individual functions from various system DLLs, while we
+    specifically want to load _many_ functions from _one_ DLL here, and
+    distinctly not a system DLL (we expect libcurl to be located outside
+    `C:\Windows\system32`, something `INIT_PROC_ADDR` refuses to work with).
+    Also, the `curl_easy_getinfo()`/`curl_easy_setopt()` functions are
+    declared as vararg functions, which `lazyload.h` cannot handle. Finally,
+    we are about to optionally override the exact file name that is to be
+    loaded, which is a goal contrary to `lazyload.h`'s design.
+
     Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
 
  ## Makefile ##
 @@ Makefile: else
- 		LAZY_LOAD_LIBCURL_OBJ = compat/lazy-load-curl.o
- 		OBJECTS += $(LAZY_LOAD_LIBCURL_OBJ)
+ 		# The `CURL_STATICLIB` constant must be defined to avoid seeing the functions
+ 		# declared as DLL imports
  		CURL_CFLAGS = -DCURL_STATICLIB
 +ifneq ($(uname_S),MINGW)
 +ifneq ($(uname_S),Windows)
@@ Makefile: else
  		ifndef CURL_LDFLAGS
  			CURL_LDFLAGS = $(eval CURL_LDFLAGS := $$(shell $$(CURL_CONFIG) --libs))$(CURL_LDFLAGS)
 
- ## compat/lazy-load-curl.c ##
+ ## compat/lazyload-curl.c ##
 @@
  #include "../git-compat-util.h"
  #include "../git-curl-compat.h"
@@ compat/lazy-load-curl.c
  #include <dlfcn.h>
 +#endif
  
+ /*
+  * The ABI version of libcurl is encoded in its shared libraries' file names.
+@@
+ 
  typedef void (*func_t)(void);
  
 +#ifndef WIN32
  #ifdef __APPLE__
- #define LIBCURL_FILE_NAME(base) base ".4.dylib"
+ #define LIBCURL_FILE_NAME(base) base "." #LIBCURL_ABI_VERSION ".dylib"
  #else
-@@ compat/lazy-load-curl.c: static func_t load_function(void *handle, const char *name)
+@@ compat/lazyload-curl.c: static func_t load_function(void *handle, const char *name)
  	*(void **)&f = dlsym(handle, name);
  	return f;
  }
 +#else
-+#define LIBCURL_FILE_NAME(base) base "-4.dll"
++#define LIBCURL_FILE_NAME(base) base "-" #LIBCURL_ABI_VERSION ".dll"
 +
 +static void *load_library(const char *name)
 +{

3: 82baabb ! 3: 2beaffb http: when loading libcurl lazily, allow for multiple SSL backends

@@ Commit message
 
     Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
 
- ## compat/lazy-load-curl.c ##
-@@ compat/lazy-load-curl.c: static curl_easy_setopt_pointer_type curl_easy_setopt_pointer_func;
+ ## compat/lazyload-curl.c ##
+@@ compat/lazyload-curl.c: static curl_easy_setopt_pointer_type curl_easy_setopt_pointer_func;
  typedef CURLcode (*curl_easy_setopt_off_t_type)(CURL *curl, CURLoption opt, curl_off_t value);
  static curl_easy_setopt_off_t_type curl_easy_setopt_off_t_func;
  
@@ compat/lazy-load-curl.c: static curl_easy_setopt_pointer_type curl_easy_setopt_p
  	if (!libcurl)
  		die("failed to load library '%s'", LIBCURL_FILE_NAME("libcurl"));
  
-@@ compat/lazy-load-curl.c: CURLcode curl_global_init(long flags)
+@@ compat/lazyload-curl.c: CURLcode curl_global_init(long flags)
  
  CURLsslset curl_global_sslset(curl_sslbackend id, const char *name, const curl_ssl_backend ***avail)
  {

4: d6289e3 ! 4: 05efe31 mingw: do load libcurl dynamically by default

@@ Metadata
  ## Commit message ##
     mingw: do load libcurl dynamically by default
 
-    This will help with Git for Windows' maintenance going forward.
+    This will help with Git for Windows' maintenance going forward: It
+    allows Git for Windows to switch its primary libcurl to a variant
+    without the OpenSSL backend, while still loading an alternate when
+    setting `http.sslBackend = openssl`.
+
+    This is necessary to avoid maintenance headaches with upgrading OpenSSL:
+    its major version name is encoded in the shared library's file name and
+    hence major version updates (temporarily) break libraries that are
+    linked against the OpenSSL library.
 
     Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho · 2023-05-10T07:29:21Z

I rebased the patches on top of the most recent merging-rebase, to avoid problems with rebasing after this PR is merged.

This compile-time option allows to ask Git to load libcurl dynamically at runtime. Together with a follow-up patch that optionally overrides the file name depending on the `http.sslBackend` setting, this kicks open the door for installing multiple libcurl flavors side by side, and load the one corresponding to the (runtime-)configured SSL/TLS backend. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This implements the Windows-specific support code, because everything is slightly different on Windows, even loading shared libraries. Note: I specifically do _not_ use the code from `compat/win32/lazyload.h` here because that code is optimized for loading individual functions from various system DLLs, while we specifically want to load _many_ functions from _one_ DLL here, and distinctly not a system DLL (we expect libcurl to be located outside `C:\Windows\system32`, something `INIT_PROC_ADDR` refuses to work with). Also, the `curl_easy_getinfo()`/`curl_easy_setopt()` functions are declared as vararg functions, which `lazyload.h` cannot handle. Finally, we are about to optionally override the exact file name that is to be loaded, which is a goal contrary to `lazyload.h`'s design. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The previous commits introduced a compile-time option to load libcurl lazily, but it uses the hard-coded name "libcurl-4.dll" (or equivalent on platforms other than Windows). To allow for installing multiple libcurl flavors side by side, where each supports one specific SSL/TLS backend, let's first look whether `libcurl-<backend>-4.dll` exists, and only use `libcurl-4.dll` as a fall back. That will allow us to ship with a libcurl by default that only supports the Secure Channel backend for the `https://` protocol. This libcurl won't suffer from any dependency problem when upgrading OpenSSL to a new major version (which will change the DLL name, and hence break every program and library that depends on it). This is crucial because Git for Windows relies on libcurl to keep working when building and deploying a new OpenSSL package because that library is used by `git fetch` and `git clone`. Note that this feature is by no means specific to Windows. On Ubuntu, for example, a `git` built using `LAZY_LOAD_LIBCURL` will use `libcurl.so.4` for `http.sslbackend=openssl` and `libcurl-gnutls.so.4` for `http.sslbackend=gnutls`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This will help with Git for Windows' maintenance going forward: It allows Git for Windows to switch its primary libcurl to a variant without the OpenSSL backend, while still loading an alternate when setting `http.sslBackend = openssl`. This is necessary to avoid maintenance headaches with upgrading OpenSSL: its major version name is encoded in the shared library's file name and hence major version updates (temporarily) break libraries that are linked against the OpenSSL library. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

To help with Git for Windows' release mechanics, Git for Windows now ships [with two variants of `libcurl`](git-for-windows/git#4410). Signed-off-by: gitforwindowshelper[bot] <gitforwindowshelper-bot@users.noreply.github.com>

…4410) As per #4350 (comment), the major block for upgrading Git for Windows' OpenSSL from v1.1 to v3 is the tricky part where such an upgrade would break `git fetch`/`git clone` and `git push` because the libcurl depends on the OpenSSL DLL, and the major version bump will _change_ the file name of said DLL. To overcome that, the plan is to build libcurl flavors for each supported SSL/TLS backend, aligning with the way MSYS2 builds libcurl, then switch Git for Windows' SDK to the Secure Channel-flavored libcurl, and teach Git to look for the specific flavor of libcurl corresponding to the `http.sslBackend` setting (if that was configured). Here is the PR to teach Git that trick.

…it-for-windows#4410) As per git-for-windows#4350 (comment), the major block for upgrading Git for Windows' OpenSSL from v1.1 to v3 is the tricky part where such an upgrade would break `git fetch`/`git clone` and `git push` because the libcurl depends on the OpenSSL DLL, and the major version bump will _change_ the file name of said DLL. To overcome that, the plan is to build libcurl flavors for each supported SSL/TLS backend, aligning with the way MSYS2 builds libcurl, then switch Git for Windows' SDK to the Secure Channel-flavored libcurl, and teach Git to look for the specific flavor of libcurl corresponding to the `http.sslBackend` setting (if that was configured). Here is the PR to teach Git that trick.

…4410) As per #4350 (comment), the major block for upgrading Git for Windows' OpenSSL from v1.1 to v3 is the tricky part where such an upgrade would break `git fetch`/`git clone` and `git push` because the libcurl depends on the OpenSSL DLL, and the major version bump will _change_ the file name of said DLL. To overcome that, the plan is to build libcurl flavors for each supported SSL/TLS backend, aligning with the way MSYS2 builds libcurl, then switch Git for Windows' SDK to the Secure Channel-flavored libcurl, and teach Git to look for the specific flavor of libcurl corresponding to the `http.sslBackend` setting (if that was configured). Here is the PR to teach Git that trick.

…it-for-windows#4410) As per git-for-windows#4350 (comment), the major block for upgrading Git for Windows' OpenSSL from v1.1 to v3 is the tricky part where such an upgrade would break `git fetch`/`git clone` and `git push` because the libcurl depends on the OpenSSL DLL, and the major version bump will _change_ the file name of said DLL. To overcome that, the plan is to build libcurl flavors for each supported SSL/TLS backend, aligning with the way MSYS2 builds libcurl, then switch Git for Windows' SDK to the Secure Channel-flavored libcurl, and teach Git to look for the specific flavor of libcurl corresponding to the `http.sslBackend` setting (if that was configured). Here is the PR to teach Git that trick.

…4410) As per #4350 (comment), the major block for upgrading Git for Windows' OpenSSL from v1.1 to v3 is the tricky part where such an upgrade would break `git fetch`/`git clone` and `git push` because the libcurl depends on the OpenSSL DLL, and the major version bump will _change_ the file name of said DLL. To overcome that, the plan is to build libcurl flavors for each supported SSL/TLS backend, aligning with the way MSYS2 builds libcurl, then switch Git for Windows' SDK to the Secure Channel-flavored libcurl, and teach Git to look for the specific flavor of libcurl corresponding to the `http.sslBackend` setting (if that was configured). Here is the PR to teach Git that trick.

dscho requested a review from rimrul May 7, 2023 22:13

dscho self-assigned this May 7, 2023

This was referenced May 7, 2023

Align libcurl with MSYS2 git-for-windows/MINGW-packages#76

Merged

Migrate away from OpenSSL v1.1 #4350

Closed

rimrul reviewed May 8, 2023

View reviewed changes

Makefile Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

compat/lazy-load-curl.c Outdated Show resolved Hide resolved

compat/lazy-load-curl.c Outdated Show resolved Hide resolved

config.mak.uname Show resolved Hide resolved

dscho force-pushed the lazy-load-curl branch from d6289e3 to 05efe31 Compare May 8, 2023 12:28

dscho requested a review from rimrul May 8, 2023 14:26

dscho force-pushed the lazy-load-curl branch from 05efe31 to 708a37a Compare May 10, 2023 07:28

dscho added 4 commits May 13, 2023 10:53

dscho force-pushed the lazy-load-curl branch from 708a37a to 2948d32 Compare May 13, 2023 08:53

dscho added this to the Next release milestone May 15, 2023

dscho merged commit 69ed455 into git-for-windows:main May 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lazy load libcurl, allowing for an SSL/TLS backend-specific libcurl #4410

Lazy load libcurl, allowing for an SSL/TLS backend-specific libcurl #4410

dscho commented May 7, 2023

dscho commented May 8, 2023

dscho commented May 10, 2023

Lazy load libcurl, allowing for an SSL/TLS backend-specific libcurl #4410

Lazy load libcurl, allowing for an SSL/TLS backend-specific libcurl #4410

Conversation

dscho commented May 7, 2023

dscho commented May 8, 2023

dscho commented May 10, 2023