ElementsProject / libwally-core

Useful primitives for wallets
Other
284 stars 136 forks source link

Python linkage enhancements #433

Closed whitslack closed 12 months ago

whitslack commented 12 months ago

(continued from #419)

whitslack commented 12 months ago

Oh, I see this is failing. It still works for Gentoo since my ebuild deletes the whole if not IS_WINDOWS block since we do the whole library build before starting the Python build, but it no longer works if the Python module is to be built without first building the library. I'll massage this some more later today.

jgriffiths commented 12 months ago

This is probably broken since the latest changes only generate a couple of files and then build with the amalgamation, instead of compiling everything twice.

In terms of a mutually acceptable change, I expect the existing build to remain unchanged (i.e. use the amalgamation) for both win and non-win. You should control the optional building with shared linkage/prebuilt libs entirely with your env var.

whitslack commented 12 months ago

As of 40c5e5ad4d3a5038785a3da09ddbfd27bd998c52, I no longer need to hack setup.py at all to achieve a holy-grail build, yet it should still work the way you're accustomed to if no special env vars are set.

jgriffiths commented 12 months ago

I'm pulling this into the build_updates branch and rebasing it back behind the v1.0 version bump. Feel free to update from that branch so when its merged this will be marked merged too, thanks.

jgriffiths commented 12 months ago

@whitslack can you confirm that https://github.com/ElementsProject/libwally-core/pull/432 works for you?

whitslack commented 12 months ago

can you confirm that #432 works for you?

@jgriffiths: 1e6a4b090b8efb1b804a1bb509ca2406cde6dfdf ticks every box on my wish list (except for one, which I hadn't previously mentioned):

The one gripe I still have, which I am okay to address in a later release, is that the libwally-core library differs depending on whether the Python module is configured to be built. Really, libwallycore.so should have no awareness of Python in any configuration, and all necessary Python glue should reside in the Python native extension library. This is especially important when we install the Python module concurrently for multiple versions of Python. For instance, on my system I have the wallycore Python module installed for both Python 3.11 and 3.12, but /usr/lib64/libwallycore.so.1.0.0 specifies a dynamic link with libpython3.12.so.1.0. I don't have a good feeling about that link when the module is loaded under Python 3.11. Really, libwallycore.so should not link with any libpython*.so at all, as such a link indicates an abstraction leak / layer violation.

jgriffiths commented 12 months ago

@whitslack thanks for testing, and your summary of matters from the gentoo POV.

I am hoping to release 1.0.0 imminently so your final point will not make this release. I am open to addressing it in a future release though, even if in an optional manner as per the setup.py changes. Feel free to raise an issue with this info for tracking.

I appreciate your patience and responsiveness in getting these changes merged.