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

Cross-Compiling libgit2 for Android: cannot find -lrt #2128

Closed
worblehat opened this issue Feb 20, 2014 · 8 comments
Closed

Cross-Compiling libgit2 for Android: cannot find -lrt #2128

worblehat opened this issue Feb 20, 2014 · 8 comments

Comments

@worblehat
Copy link
Contributor

I'd like to use libgit2 in an Android application. So I followed the build instuctions in README.md. I generated an arm standalone toolchain for the NDK and created the toolchain CMake-file. When building libgit2 I get an error, because the linker can't find librt:

ld: error: cannot find -lrt

I don't know much about librt, but as far as I understand, the NDK does not include librt, but instead integrates some of it's functionality in Android's libc.
So my approach to handle this error, was to edit CMakelists.txt, to not link to librt when CMAKE_SYSTEM_VERSION is set to "Android":

ELSEIF(CMAKE_SYSTEM_NAME MATCHES "Linux")
    IF(CMAKE_SYSTEM_VERSION MATCHES "Android")
        # nothing...
    ELSE()
        TARGET_LINK_LIBRARIES(${target} rt)
        SET(LIBGIT2_PC_LIBS "${LIBGIT2_PC_LIBS} -lrt" PARENT_SCOPE)
    ENDIF()
 ENDIF()

This compiled without problems and I can actually link to the libgit2.so in my Android App.

I don't have any experience with libgit2 and NDK, so I don't really know if this is the right way or if this will cause any problems. What do you think?

@ben
Copy link
Member

ben commented Feb 20, 2014

It sounds like this would be a good fix for the library. Can you submit that change as a pull request? That way you can make sure it works, and you'll get credit for the fix. 😄

One suggestion to avoid the nested IF:

ELSEIF(CMAKE_SYSTEM_NAME MATCHES "Linux" AND NOT CMAKE_SYSTEM_VERSION MATCHES "Android")
    TARGET_LINK_LIBRARIES(${target} rt)
    SET(LIBGIT2_PC_LIBS "${LIBGIT2_PC_LIBS} -lrt" PARENT_SCOPE)
ENDIF()

@arrbee
Copy link
Member

arrbee commented Feb 20, 2014

Actually, I'd rather detach this check from matching the OS and instead directly test if we need -lrt to get access to clock_gettime. I think we want something like:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 6f2a2bb..723518a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -78,7 +78,10 @@ FUNCTION(TARGET_OS_LIBRARIES target)
    ELSEIF(CMAKE_SYSTEM_NAME MATCHES "(Solaris|SunOS)")
        TARGET_LINK_LIBRARIES(${target} socket nsl)
        SET(LIBGIT2_PC_LIBS "${LIBGIT2_PC_LIBS} -lsocket -lnsl" PARENT_SCOPE)
-   ELSEIF(CMAKE_SYSTEM_NAME MATCHES "Linux")
+   ENDIF()
+
+   CHECK_LIBRARY_EXISTS(rt clock_gettime "time.h" NEED_LIBRT)
+   IF(NEED_LIBRT)
        TARGET_LINK_LIBRARIES(${target} rt)
        SET(LIBGIT2_PC_LIBS "${LIBGIT2_PC_LIBS} -lrt" PARENT_SCOPE)
    ENDIF()

I didn't actually test that on Android, but I think it might work.

@worblehat
Copy link
Contributor Author

@ben Thanks for your suggestion. I was note sure how to negate the 'Matches'.

@arrbee This seems to work. It compiled successfully with my Android NDK toolchain. I just had to add an include, to be able to use the CHECK_LIBRARY_EXISTS function:

INCLUDE(CheckExistsLibrary)

@worblehat
Copy link
Contributor Author

I just created a pull request.
(I'm not familiar with this kind of workflow, so please tell me if I don't meet the convention.)

@arrbee
Copy link
Member

arrbee commented Feb 21, 2014

@worblehat Looks good. Let's make sure that the tests all pass on your Pull Request (Travis CI will run them for us and label the PR - you don't have to do anything), but it seems fine to me.

@arrbee
Copy link
Member

arrbee commented Feb 21, 2014

Okay @worblehat - I merged your pull request. Thanks for putting it together. ✨

@arrbee arrbee closed this as completed Feb 21, 2014
@alexband
Copy link

@xtao I remember we have the same -lrt issue once, see if we update libgit2 we can drop the workaround before?

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

No branches or pull requests

5 participants
@ben @arrbee @alexband @worblehat and others