Created attachment 121683 [details] Don't link to rt when building for Android Patch fixes linker issue when building for Android... [ 36%] Linking C shared library ../lib/libdbus-1.so F:/android/android-ndk-r10e/toolchains/arm-linux-androideabi-4.9/prebuilt/windows-x86_64/bin/../lib/gcc/arm-linux-androideabi/4.9/../../../../arm-linux-androideabi/bin/ld.exe: error: cannot find -lrt collect2.exe: error: ld returned 1 exit status make[2]: *** [lib/libdbus-1.so] Error 1 make[1]: *** [dbus/CMakeFiles/dbus-1.dir/all] Error 2 make: *** [all] Error 2
The CMake build system is intended for builds that run on either Windows or GNU/Linux. I would recommend the Autotools build system for portable builds and/or cross-compilation. Building on a Windows build system, cross-compiling for an Android host system, using the CMake build system is way outside anything we've ever actively supported.
(In reply to Simon McVittie from comment #1) > The CMake build system is intended for builds that run on either Windows or > GNU/Linux. I would recommend the Autotools build system for portable builds > and/or cross-compilation. See also Bug #90687, which is targeting QNX rather than Android but the principles are the same. In particular, <https://bugs.freedesktop.org/show_bug.cgi?id=90687#c5> and <https://bugs.freedesktop.org/show_bug.cgi?id=90687#c9> describe the approach we should take for -lrt (and, for QNX, -lsocket). https://bugs.freedesktop.org/show_bug.cgi?id=90687#c11 suggests that something like this might work: # for clock_getres() on e.g. GNU/Linux (but not Android) find_library(LIBRT rt) if(!LIBRT) target_link_libraries(dbus-1 ${LIBRT}) endif() # for socket() on QNX find_library(LIBSOCKET socket) if(LIBSOCKET) target_link_libraries(dbus-1 ${LIBSOCKET}) endif() I know you don't need the socket part yourself, but if you add this code for -lsocket at the same time as -lrt, then normal GNU/Linux builds will test the "we found this library" code path for -lrt, and the "we didn't find this library" code path for -lsocket. Code that isn't tested doesn't work, except by mistake, so regularly testing both is valuable. (Your patch also doesn't link to ${CMAKE_THREAD_LIBS_INIT} on Android - but that variable is never set anyway, so it should just be deleted altogether, and not used on GNU/Linux either.)
Hi Simon, thank you for responding so quickly to my reports. Yes, I am cross compiling for Android from Windows using Google's toolchain (https://code.google.com/archive/p/android-cmake/). We appreciate Google's work here as it saves us from writing new build system code just for Android. DBus is a fantastic project that we would like to use without having to patch the source locally before building. Your help in this regard is much appreciated. What could help is if we could not try to link against rt unless the package is found, like: find_library(LIBRT rt) if(LIBRT) target_link_libraries(dbus-1 ${LIBRT}) endif() Is the "if(!LIBRT)" a typo?
(In reply to Eric from comment #3) > Is the "if(!LIBRT)" a typo? Er, yes, I meant if(LIBRT), similar to the LIBSOCKET one. Please try that (or if it doesn't work, adjust it until it does) and see whether that solves this for you.
(In reply to Simon McVittie from comment #2) > > # for clock_getres() on e.g. GNU/Linux (but not Android) > find_library(LIBRT rt) > if(!LIBRT) According to https://cmake.org/cmake/help/v3.0/command/find_package.html?highlight=_found this need to be if(LIBRT_FOUND) > find_library(LIBSOCKET socket) > if(LIBSOCKET) if(LIBSOCKET_FOUND)
Yes, this solves this for me and is a better solution - thank you.
(In reply to Eric from comment #6) > this solves this for me and is a better solution What is the "this" that solves it for you? If you have a revised patch (perhaps based on Comment #2, Comment #3, Comment #5), please propose it here - I'd like to merge a change that does the right thing for -lrt and -lsocket. If I have to guess at the precise details of how you adapted my pseudocode into a specific patch, sorry but it isn't going to get merged.
(In reply to Ralf Habacker from comment #5) > if(LIBRT_FOUND) > if(LIBSOCKET_FOUND) Sorry, I was wrong and Simon was correct. My comment is valid for find_package.
Created attachment 125831 [details] [review] Do not link dbus-1 and dbus-internal against librt and libsocket if not present. This fixes a cross link issue on windows for android. Bug:
Comment on attachment 121683 [details] Don't link to rt when building for Android superseeded
Comment on attachment 125831 [details] [review] Do not link dbus-1 and dbus-internal against librt and libsocket if not present. Review of attachment 125831 [details] [review]: ----------------------------------------------------------------- The code changes are fine, the commit message could do with some improvement. > Do not link dbus-1 and dbus-internal against librt and libsocket if not present That's not actually what this patch does. It is making two changes: * Previously, we linked to librt unconditionally. Now, only link it if present. * Previously, we never linked to libsocket. Now, link it if present. I'd prefer those to be two separate commits. The first is necessary when building with CMake for a Unix platform that does not have -lrt, such as Android. The second is necessary when building with CMake for a Unix platform where functions like recv() are in a separate -lsocket, like QNX. > This fixes a cross link issue on windows for android. For the librt change: "This lets us build using CMake for an Android host, for instance on Windows" or something. Whether the build platform is Windows is actually not relevant. For the addition of socket, similarly: "This lets us build using CMake for a QNX host".
Created attachment 125851 [details] [review] Fix building with CMake for a Unix platform that does not have -lrt, such as Android. Bug:
Created attachment 125852 [details] [review] Fix building with CMake for a Unix platform where functions like recv() are in a separate -lsocket, like QNX. Bug:
Comment on attachment 125831 [details] [review] Do not link dbus-1 and dbus-internal against librt and libsocket if not present. superseeded
Separate patches look good, thanks.
*** Bug 90687 has been marked as a duplicate of this bug. ***
Comment on attachment 125852 [details] [review] Fix building with CMake for a Unix platform where functions like recv() are in a separate -lsocket, like QNX. submitted to master
Comment on attachment 125851 [details] [review] Fix building with CMake for a Unix platform that does not have -lrt, such as Android. submitted to master
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.