dynamically load libcurl #3009

MartinNowak · 2015-02-20T03:47:26Z

avoids issues with versioned symbols on different platforms
lazy loading/initialization of curl library
quick unittests now also need $(LINKDL)

Issue 13324 – dynamically load libcurl at runtime

quickfur · 2015-03-19T18:36:31Z

Briefly looked over it, the overall implementation seems OK. Don't have time right now to look at it in detail, though. Hopefully other reviewers will chime in...

JakobOvrum · 2015-03-21T15:55:06Z

std/net/curl.d

+        else
+            static assert(0, "unimplemented");
+
+        enforce!CurlException(handle !is null, "Failed to load libcurl.");


I'm worried that this error will be thrown at people currently linking with curl libraries with non-standard or fringe names, and they won't know which names are tried without looking at the source code. Maybe it's a good idea to include the list of tried names in the exception message? If it's going to be a breaking change for them, we should at least make it as comfortable as possible.

Mmh, I spent some effort trying to find all relevant names. How would someone rename their libcurl.
I might look through a few more implementations, e.g. perl to see what works.

I added one name libcurl-nss.so.4 which is one of the libcurl flavors on ubuntu, couldn't find anything relevant besides this.

MartinNowak · 2015-03-30T12:09:50Z

I shortly enabled the std.net.curl tests to see whether I was missing something, but they pass on all platforms.
MartinNowak@090aaad
https://auto-tester.puremagic.com/pull-history.ghtml?projectid=1&repoid=3&pullid=3009

CyberShadow · 2015-04-04T03:52:20Z

std/net/curl.d

-    {
-        // initialize early to prevent thread races
-        enforce!CurlException(!curl_global_init(CurlGlobal.all),
-                                "Couldn't initialize libcurl");


~~We no longer call curl_global_init anywhere explicitly, is this OK?~~

It is called above: _api.global_init

CyberShadow · 2015-04-04T05:11:29Z

Not 100% sure about this... this change means that it will be impossible to statically link libcurl by substituting the import library with a static one.

MartinNowak · 2015-04-04T18:54:57Z

I could try to load the symbols from the binary itself, but for that to work, you need to use the export-dynamic switch, not sure if that works on windows.

CyberShadow · 2015-04-05T00:43:06Z

lazy loading/initialization of curl library

Lazy loading has its disadvantages (sometimes it's better to fail fast).

avoids issues with versioned symbols on different platforms

This is the real reason for this change as far as I can see. Which platforms are affected?

CyberShadow · 2015-04-05T00:44:48Z

but for that to work, you need to use the export-dynamic switch

Does this export all public symbols or something? If so, that would conflict with one of the use cases for why you'd want to statically link libcurl (self-contained closed-source application).

MartinNowak · 2015-04-08T09:30:03Z

We could add weak undefined references to curl and check whether those are defined.
Do you think static linking of curl is an important enough use-case to warrant the effort?

MartinNowak · 2015-04-13T14:39:25Z

I could try to use undefined weak references and initialize pointers from that when curl is statically linked, not sure if that'd work on Windows.
Do you really think it's worth the effort @CyberShadow?

CyberShadow · 2015-04-13T15:08:54Z

I don't know.

How important is the versioned symbol problem?

Can we do this only for affected systems?

MartinNowak · 2015-04-13T18:37:14Z

We also have a link order issue, because dmd currently doesn't allow to add libraries after phobos.
Let's try the weak ref stuff, shouldn't be too hard.

MartinNowak · 2015-05-18T15:30:27Z

There is no easy way to get weakly linked but undefined symbols in D. Function declarations in templates are strong undefined. So we'll have to add a C file, hope that dmc supports weak linkage.

dnadlinger · 2015-06-11T06:30:58Z

Can we go ahead with this in the meantime? LDC is hit pretty hard with the linker order issue right now, and I'd rather not add -lcurl as a default dependency for every built executable. I might actually merge this into the LDC Phobos fork for the next release.

This is the only sensible solution until e.g. dlang/phobos#3009 is merged, which makes Phobos load libcurl dynamically. The issue with the current situation is is that the user cannot even manually link libcurl if linker errors occur, because it will appear before libphobos2-ldc in the linker command line. GitHub: Fixes ldc-developers#906.

DmitryOlshansky · 2015-08-09T14:53:10Z

ODBC will have the same issue once we have D version of std.db.odbc of some such.

MartinNowak · 2015-08-19T15:24:31Z

Not sure if dmc actually supports weak symbols. I think the simpler solution is to try to dynamically load the curl symbols from the executable.
That way it's still possible to ship a single binary.

MartinNowak · 2015-08-20T00:51:52Z

done, now also with support for linking curl
still waiting for the tests to pass

MartinNowak · 2015-08-20T09:37:14Z

ODBC will have the same issue once we have D version of std.db.odbc of some such.

Let's hope we can prevent the mistake to add ODBC.

MartinNowak · 2015-08-20T09:38:28Z

std.net.curl tests pass though there is an unrelated and already existing 32-bit bug.
https://auto-tester.puremagic.com/pull-history.ghtml?projectid=1&repoid=3&pullid=3009

DmitryOlshansky · 2015-08-21T07:12:09Z

@MartinNowak - what needs to be done to be able to import from the executable itself? Static linking wouldn't suffice I guess?

MartinNowak · 2015-08-21T14:58:31Z

It should work right away when linking with an import library for a curl.dll or with libcurl.so on Posix, b/c loading a symbol from the exe does search for symbols in dependent libraries (not 100% sure if that works on Windows).

For linking with a static curl library you need the export declarations on Windows (either using a DEF file or using the static lib itself) and --export-dynamic on Posix.

MartinNowak · 2015-08-22T11:30:36Z

Updated with detailed changelog entry and manual on how to statically link against curl.
Also improved the error message to list all the library names that were attempted.

CyberShadow · 2015-08-22T13:30:33Z

Pretty cool solution to allow linking libcurl statically on Windows, wouldn't have thought of that. 👍

- avoids issues with versioned symbols on different platforms - lazy loading/initialization of curl library - fix Issue 13324 - try to load curl from the exe itself before loading a shared library thus allowing to link against a different or a static curl library

MartinNowak · 2015-08-22T14:29:11Z

Pretty cool solution to allow linking libcurl statically on Windows, wouldn't have thought of that.

Well some very security focused people might not like it, but the weak references weren't an option with DMC.

MartinNowak · 2015-08-22T14:31:29Z

The nice side effect is that you can link with any shared library on Posix, e.g. a hypothetical libcurl-botan.so and it will work, even without --export-dynamic.

dnadlinger · 2015-08-22T16:03:26Z

Auto-merge toggled on

dynamically load libcurl

MartinNowak · 2015-08-22T23:45:59Z

Finally

s-ludwig · 2015-09-21T12:51:09Z

Is it intended that this now still requires to manually pass a library flag (-ldl) to the compiler? Wouldn't this just shift the issue, or is the idea that -ldl is simply always available and thus less likely to cause problems? There is currently dlang/dub#660 open, but if libdl stays as an additional required (manual) dependency, it will again break all builds for projects using std.net.curl.

MartinNowak · 2015-09-26T02:18:50Z

Already fixed dlang/dmd#5084.

s-ludwig · 2015-09-26T06:15:55Z

Okay, thanks, then the other guy probably just tested with a slightly older version of GIT master.

MartinNowak force-pushed the loadCurlAtRuntime branch 3 times, most recently from 9d563d4 to fd5465e Compare February 20, 2015 04:03

JakobOvrum reviewed Mar 21, 2015
View reviewed changes

MartinNowak force-pushed the loadCurlAtRuntime branch from fd5465e to 090aaad Compare March 30, 2015 10:58

MartinNowak force-pushed the loadCurlAtRuntime branch from 090aaad to 2b1b2ba Compare March 30, 2015 12:10

CyberShadow reviewed Apr 4, 2015
View reviewed changes

This was referenced May 14, 2015

fix Issue 14535 - std.net.curl.CurlException should include status line #3257

Merged

keep bool overloads #3290

Closed

Small program that tells you how hip D is MartinNowak/YourCodeHere#1

Merged

CyberShadow mentioned this pull request Jun 11, 2015

Add etc.c.odbc #3398

Merged

dnadlinger mentioned this pull request Jun 14, 2015

Always link libcurl when Phobos is linked ldc-developers/ldc#977

Merged

MartinNowak force-pushed the loadCurlAtRuntime branch from 2b1b2ba to fcfe3a7 Compare August 20, 2015 00:49

MartinNowak force-pushed the loadCurlAtRuntime branch 2 times, most recently from da0e762 to 0e1a04e Compare August 20, 2015 09:34

MartinNowak force-pushed the loadCurlAtRuntime branch 3 times, most recently from 8162aef to 30ec62a Compare August 22, 2015 11:29

MartinNowak force-pushed the loadCurlAtRuntime branch from 30ec62a to 592ce24 Compare August 22, 2015 14:28

dnadlinger added a commit that referenced this pull request Aug 22, 2015

Merge pull request #3009 from MartinNowak/loadCurlAtRuntime

de08b92

dynamically load libcurl

dnadlinger merged commit de08b92 into dlang:master Aug 22, 2015

dnadlinger mentioned this pull request Aug 22, 2015

fix Issue 13007 - Wrong x86 code: long negate dlang/dmd#4920

Merged

MartinNowak deleted the loadCurlAtRuntime branch August 22, 2015 23:45

jpf91 mentioned this pull request Mar 31, 2016

Linking adjustments (for MinGW and system zlib) D-Programming-GDC/gdc#181

Closed

Uh oh!

dynamically load libcurl #3009

dynamically load libcurl #3009

Uh oh!

Conversation

MartinNowak commented Feb 20, 2015

Uh oh!

quickfur commented Mar 19, 2015

Uh oh!

JakobOvrum Mar 21, 2015

Choose a reason for hiding this comment

Uh oh!

MartinNowak Mar 22, 2015

Choose a reason for hiding this comment

Uh oh!

MartinNowak Mar 30, 2015

Choose a reason for hiding this comment

Uh oh!

MartinNowak commented Mar 30, 2015

Uh oh!

CyberShadow Apr 4, 2015

Choose a reason for hiding this comment

Uh oh!

CyberShadow commented Apr 4, 2015

Uh oh!

MartinNowak commented Apr 4, 2015

Uh oh!

CyberShadow commented Apr 5, 2015

Uh oh!

CyberShadow commented Apr 5, 2015

Uh oh!

MartinNowak commented Apr 8, 2015

Uh oh!

MartinNowak commented Apr 13, 2015

Uh oh!

CyberShadow commented Apr 13, 2015

Uh oh!

MartinNowak commented Apr 13, 2015

Uh oh!

MartinNowak commented May 18, 2015

Uh oh!

dnadlinger commented Jun 11, 2015

Uh oh!

DmitryOlshansky commented Aug 9, 2015

Uh oh!

MartinNowak commented Aug 19, 2015

Uh oh!

MartinNowak commented Aug 20, 2015

Uh oh!

MartinNowak commented Aug 20, 2015

Uh oh!

MartinNowak commented Aug 20, 2015

Uh oh!

DmitryOlshansky commented Aug 21, 2015

Uh oh!

MartinNowak commented Aug 21, 2015

Uh oh!

MartinNowak commented Aug 22, 2015

Uh oh!

CyberShadow commented Aug 22, 2015

Uh oh!

MartinNowak commented Aug 22, 2015

Uh oh!

MartinNowak commented Aug 22, 2015

Uh oh!

dnadlinger commented Aug 22, 2015

Uh oh!

MartinNowak commented Aug 22, 2015

Uh oh!

s-ludwig commented Sep 21, 2015

Uh oh!

MartinNowak commented Sep 26, 2015

Uh oh!

s-ludwig commented Sep 26, 2015

Uh oh!

Uh oh!