bitcoin: `libbitcoinkernel`: Building `mingw-w64` dll's broken

We’re unable to build a dll for libbitcoinkernel right now because of unique problems arising out of the combination of mingw-w64 + winpthread + libtool + dll's

https://github.com/bitcoin/bitcoin/blob/466c6161505f0c9c68e4c4f41e3e40b8660d3a7a/src/Makefile.am#L816-L822

Various methods have been proposed and we should discuss here.

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Comments: 22 (22 by maintainers)

Commits related to this issue

Most upvoted comments

Upstream acknowledged the problem and committed a fix that goes even further than my proposed patch: https://sourceforge.net/p/mingw-w64/mingw-w64/ci/489520900fb6853c8be97e9d0214f39a77c846d9/

(Funny enough, they went back and forth a few times to arrive at that fix, so I suppose this is quite subtle indeed)

So the good news is: this is now fixed upstream and we don’t have to opt-in, it’ll just work when building against the next release. The bad news is: we’ll have to carry a work-around until this trickles down to the majority of mingw toolchains, which will take several years.

libsecp256k1 actually has the opposite problem: leaking exported symbols into static libraries.

I’ve PR’d a fix for that upstream: https://github.com/bitcoin-core/secp256k1/pull/1105

Without that fix, the windows dll links successfully, but exports only the libsecp symbols due to ld’s behavior (emphasis mine):

–export-all-symbols If given, all global symbols in the objects used to build a DLL will be exported by the DLL. Note that this is the default if there otherwise wouldn’t be any exported symbols.

So basically, because libsecp256k1’s symbols are exported, nothing else gets exported. By fixing that leak we get back to the default of all exports, which is what we’re after (for now).

I actually think it’s entirely possible that building a convenience library and then an actual library out of the convenience library could be the way to do it like in https://github.com/bitcoin/bitcoin/pull/24994. We shall see.

I don’t see why we should be refactoring our code, that works everywhere else, and adding more libraries, to work around a problem that occurs for a single OS, and in some part, only in certain build configurations. Especially if we know that it’s libtool that is “broken”, it’d seem better for us to just use a single, well documented, scoped to the Windows use of libtool workaround, as opposed to changing our source code to accommodate its quirkiness.

Note that #24994 alone seemingly doesn’t fix all the issues, i.e #24972, so even if we went ahead with either/both changes, we’re still going to continue to carry the windows-only libtool workarounds we’ve already got in our build system, which I’d say is the worst of both worlds (code refactoring + still having windows libtool workarounds). I’d rather add one more (well documented) libtool workaround to the pile, and hope that this may all become a moot point some time into the future (cmake 🦆s). Interested to see what appears on the mailing list upstream.

I’ve opened a pull-request upstream to add a fix for this: https://sourceforge.net/p/mingw-w64/mingw-w64/merge-requests/23/

I’ll wait a day or two and see if they respond before going any further on this.

@hebasto Sorry, that wasn’t clear from the change.

Without this hack, they contain:-DDLL_EXPORT -DPIC. So the override here simply removes the DLL_EXPORT define.