oxen-io / oxen-mq

Communications layer used for both the Oxen storage server and oxend
https://oxen.io
BSD 3-Clause "New" or "Revised" License
19 stars 35 forks source link

Fix libzmq library linking #74

Closed XutaxKamay closed 2 years ago

XutaxKamay commented 2 years ago

If I understood correctly, I suppose this was a logic error, where BUILD_SHARED_LIBS=ON should be forcing to use the bundled version and build libzmq and link statically, and OFF where it should be choosing the libzmq system's version, if not found then force from bundled version.

This fixes on my machine my builds perfectly at least, this will permit me to advance on my Gentoo ebuild.

XutaxKamay commented 2 years ago

oops I haven't seen this one,

as for why the Debian SID build with libc++ failed I think it is due because it uses gcc's stdlib headers somewhere rather than LLVM's ones somehow where both implementations are different, or it is still linking with libstdc++ even though -stdlib=libc++ is given, something like that. Running with clang++-13 -v might give more information.

On my side I made a little test with:

cmake .. -G Ninja -DCMAKE_CXX_FLAGS=-fdiagnostics-color=always -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER=clang-13 -DCMAKE_CXX_COMPILER=clang++-13 -DCMAKE_CXX_FLAGS=-stdlib=libc++ -DCMAKE_EXE_LINKER_FLAGS=-fuse-ld=lld -DCMAKE_MODULE_LINKER_FLAGS=-fuse-ld=lld -DCMAKE_SHARED_LINKER_FLAGS=-fuse-ld=lld -DCMAKE_STATIC_LINKER_FLAGS=-fuse-ld=lld

EDIT: no, it does not work, I've forgot to apply my own patch, let me see why exactly

okay I get the same issue as before when trying to build Lokinet, I'm getting a bug where the linker fails to find libzmq.a when it tries to statically link because I inverted the logic, one second

majestrate commented 2 years ago

if i recall BUILD_SHARED_LIBS is indicating if we want to build liboxenmq as a shared library or not. i dont think building with its own zmq makes sense if BUILD_SHARED_LIBS is set to ON

XutaxKamay commented 2 years ago

Alright, this was an issue on my system, my bad; to be clear it was because somehow I got BUILD_SHARED_LIBS=ON and BUILD_SHARED_LIBS=OFF mixed, but it is gone now, I'll be closing the pull request

EDIT: I still got an issue with statically linking, but that will be for another pull request once I find the real issue

majestrate commented 2 years ago

it is likely something could be potato in the cmake. i have seen this happen before off of debian build enviroments.

XutaxKamay commented 2 years ago

I just did a quick test build again in case if you monitor the issue, basically my linker is trying to link with libzmq/lib/libzmq.a when it should be libzmq/lib64/libzmq.a when -DBUILD_SHARED_LIBS=OFF

majestrate commented 2 years ago

oh god it's that issue again, i think we have the required cmake runes to fix this. let me dust off my necrocmakecon

majestrate commented 2 years ago

aha, when referencing lib/ we should be using ${LIBDIR} from GNUInstallDirs

XutaxKamay commented 2 years ago

ah rip, yeah I just saw this now in LocalLibzmq.cmake should be easy to fix then

insert cmake nightmare meme here

or well, just the whole GNU filesystem ...

XutaxKamay commented 2 years ago

Should I wait for your fix or I make a pull request? Also ${LIBDIR} does not work, but ${CMAKE_INSTALL_LIBDIR} does... The documentation is not really clear about that, and I got that by looking at the fucking cmake core module, there is a serious problem on the doc or I'm the one being dumb sigh

Ah right, by looking correctly at the doc it says: CMAKEINSTALL<dir>

where <dir> can be LIBDIR

They should have been fine by just giving CMAKE_INSTALL_LIBDIR in the first place

majestrate commented 2 years ago

you can reopen this PR with the updated fixes applied.

XutaxKamay commented 2 years ago

Does this sound good?

majestrate commented 2 years ago

i'll let @jagerman review it as a final pass but it looks fine to me.

jagerman commented 2 years ago

Looks good to me.

jagerman commented 2 years ago

Though:

This fixes on my machine my builds perfectly at least, this will permit me to advance on my Gentoo ebuild.

Shouldn't you just depend on the system libzmq? The bundled version is there as a sort of hack for environments that are afraid of/unable to link to system libs.

XutaxKamay commented 2 years ago

Though:

This fixes on my machine my builds perfectly at least, this will permit me to advance on my Gentoo ebuild.

Shouldn't you just depend on the system libzmq? The bundled version is there as a sort of hack for environments that are afraid of/unable to link to system libs.

That was exactly my issue I was basically wanting to build from system libzmq, I thought it was first a logic error (it was not I had just some confusions on SHARED_LIBS=ON/OFF), the problem was more just my cmake configuration that has failed multiple times for some reasons, though once I did setup my env correctly, it built correctly with system libs, but not with the bundled version in some systems (this is not really necessary indeed, but in case that the libzmq system version is lower it might turn useful in some situations)

Hence why, this small patch.