Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix static builds for Ubuntu 22.04 #8564

Merged

Conversation

LocalMonero
Copy link
Contributor

Link libbsd, libmd, libprotokit in CMakeLists.txt for ZMQ to fix static builds for Ubuntu 22.04

Link libbsd, libmd, libprotokit for ZMQ to fix static builds for Ubuntu 22.04
@jeffro256
Copy link
Contributor

How were you linking before so that it didn't work? I'm using Ubuntu 22.04 and it works fine for me on master.

@LocalMonero
Copy link
Contributor Author

LocalMonero commented Sep 13, 2022

@jeffro256 We did it exactly as described in the README.md, apart from also building and installing unbound and its dependency expat from source, which is now an (undocumented) standard procedure on Ubuntu after the project decided to stop vendoring unbound. You were running make release-static and it worked on your clean Ubuntu 22.04 install? For us, we got

[ 98%] Linking CXX executable ../../bin/monerod
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libzmq.a(libzmq_la-ws_engine.o): in function `zmq::ws_engine_t::server_handshake()':
(.text+0x1a6b): undefined reference to `strlcpy'
/usr/bin/ld: (.text+0x1ab9): undefined reference to `strlcpy'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libzmq.a(libzmq_la-ws_engine.o): in function `zmq::ws_engine_t::client_handshake()':
(.text+0x24a5): undefined reference to `strlcpy'
/usr/bin/ld: (.text+0x24cc): undefined reference to `strlcpy'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libnorm.a(normApi.cpp.o): in function `NormInstance::SetCacheDirectory(char const*)':
(.text+0xe0): undefined reference to `ProtoDispatcher::SuspendThread()'
/usr/bin/ld: (.text+0x153): undefined reference to `ProtoDispatcher::ResumeThread()'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libnorm.a(normApi.cpp.o): in function `NormInstance::PurgeObjectNotifications(void const*)':
(.text+0x1b9): undefined reference to `ProtoList::Iterator::Iterator(ProtoList&, bool)'
/usr/bin/ld: (.text+0x1da): undefined reference to `ProtoList::Iterator::GetNextItem()'
/usr/bin/ld: (.text+0x201): undefined reference to `ProtoList::Iterator::~Iterator()'
/usr/bin/ld: (.text+0x237): undefined reference to `ProtoList::Remove(ProtoList::Item&)'
/usr/bin/ld: (.text+0x244): undefined reference to `ProtoList::Append(ProtoList::Item&)'
/usr/bin/ld: (.text+0x261): undefined reference to `ProtoList::Append(ProtoList::Item&)'
...

Running make works. It's just make release-static that fails without the CMakeLists.txt modifications.

@jeffro256
Copy link
Contributor

jeffro256 commented Sep 18, 2022

Hi @LocalMonero, sorry it took so long to get back to you. I was trying out a bunch of Ubuntu installations trying to reproduce the linking errors that you were getting but I was not able to. From a clean installation of Ubuntu Server 22.04 on a VM, I 1) did sudo apt update && sudo apt upgrade && sudo apt install <all monero dependencies> 2) cloned the unbound repo and built and installed from source 3) cloned the monero repo and ran make release-static. I was able to build release-static without any errors. Is there anything else you can think of that you might have done to your install of Ubuntu 22 that would cause this error?

@LocalMonero
Copy link
Contributor Author

LocalMonero commented Sep 18, 2022

@jeffro256 we didn't clone and build MiniUPnP, only libunbound.

@jeffro256
Copy link
Contributor

jeffro256 commented Sep 19, 2022

Sorry, that's what I meant to say: I built and installed libunbound from source. Do you have the package libunbound-dev installed? If you do, what happens when you sudo apt remove that package, then do a make clean and rebuild monero?

Edit: I would do this on an installation which can be wiped if needed. You can sudo apt install libunbound-dev after you do the above commands to (probably) fix any missing dependencies.

@selsta
Copy link
Collaborator

selsta commented Sep 19, 2022

I needed this patch to compile release-static on a fresh Ubuntu 22.04 machine.

@jeffro256
Copy link
Contributor

Does this work without removing the libunbound-dev package?

@selsta
Copy link
Collaborator

selsta commented Sep 19, 2022

I assume it depends where you install the custom compiled unbound and which path you tell CMake.

Copy link
Contributor

@jeffro256 jeffro256 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes when libunbound's dependencies are linked statically through the --enable-static --disable-shared flags when ./configureing.

@jtgrassie
Copy link
Contributor

Shouldn't all these checks be wrapped in if(STATIC) so they only apply to static builds?

@LocalMonero
Copy link
Contributor Author

LocalMonero commented Sep 20, 2022

@jeffro256 building with sudo apt install libunbound-dev doesn't work, static or not. Only self-compiled libunbound works. No libunbound-dev installed.

@jeffro256
Copy link
Contributor

@LocalMonero Right. I was saying that when the libunbound-dev package is installed, our CMake rules for finding libunbound might have linked against the wrong library (I don't know if they did or not). For those who are linking Monero statically, it is probably best to remove that package, as long as you don't have other packages which depend on it.

@jeffro256
Copy link
Contributor

Shouldn't all these checks be wrapped in if(STATIC) so they only apply to static builds?

Probably? It probably shouldn't negatively affect non-static builds, but it would be likely be the better option, at least so the code is self-documenting in that this is a patch for static builds.

@busyboredom
Copy link

This fixes builds for me too (also ubuntu 22.04). Would be nice if we could get this merged so master would build out-of-the-box again.

@selsta
Copy link
Collaborator

selsta commented Oct 20, 2022

@busyboredom it's in the merge queue already

@jeffro256
Copy link
Contributor

@selsta what do you think of @jtgrassie 's comment?

@jtgrassie
Copy link
Contributor

@jeffro256

Probably? It probably shouldn't negatively affect non-static builds, but it would be likely be the better option, at least so the code is self-documenting in that this is a patch for static builds.

My general stance is this: If it's not needed (based on make options), don't link it.

@selsta
Copy link
Collaborator

selsta commented Oct 20, 2022

We already link to libraries that are only needed for static, so it's fine to change it but I would do it in a separate PR later.

@luigi1111 luigi1111 merged commit 05ccf68 into monero-project:master Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants