search

jedisct1 / libsodium

A modern, portable, easy to use crypto library.
https://libsodium.org
Other
12.06k stars 1.72k forks source link

convenience android multiarch compilation #1140

Closed ReenigneCA closed 2 years ago

ReenigneCA commented 2 years ago

After a good deal of experimenting I have revised the scripts I submitted for https://github.com/jedisct1/libsodium/pull/1139 such that they generate a directory structure that is easily useable with CMAKE. I confused myself duplicating structures I'd seen other places and then thinking some of my libraries defaulted to that structure... It turns out that CMAKE will search in subdirectories based on the name of the compiler used, with the exception of armv7a where it requires the directory to be named arm-linux-androideabi. setting a PREFIX in android.sh and running it will result in a directory structure like:

${PREFIX/include}
sodium
sodium.h

${PREFIX}/lib
aarch64-linux-android:
libsodium.a  libsodium.la  libsodium.so  pkgconfig

arm-linux-androideabi:
libsodium.a  libsodium.la  libsodium.so  pkgconfig

i686-linux-android:
libsodium.a  libsodium.la  libsodium.so  pkgconfig

x86_64-linux-android:
libsodium.a  libsodium.la  libsodium.so  pkgconfig

This structure can then be used for all 4 architectures using CMAKE via:

list(APPEND CMAKE_FIND_ROOT_PATH "${PREFIX}")
find_library(libsodium NAMES sodium REQUIRED)

and adding libsodium as a target_link_library.

The android.sh build script now echos instructions on how to do this after it runs. That also displays an alternate form for static linking which requires logic for x86. x86 static linking won't work with the current compiles because of text relocation rules. (I tried naively setting -fPIC in the CFLAGS for x86 but it appears that the problem is likely with assembly files. https://android.googlesource.com/platform/bionic/+/master/android-changes-for-ndk-developers.md )

I also cleaned up the various variables. The ARCH variable wasn't used at all so I removed it. I left the march flag hard coded in the scripts for each specific architecture as it wasn't meaningful outside that script. TARGET_ARCH wasn't necessary at all. I added a TARGET_ARCH_DIR variable which is normally the same as HOST_COMPILER except for armv7a. Additionally I added a commented out line in android.sh to make it clear how to disable the minimal build which is the default (the original scripts had minimal as the default.)

I chose to target CMAKE as that's what Android Studio uses and I think this layout optimizes for simple compiling. I like to keep my android libs separate from my main ones but I believe setting /usr/local as the PREFIX would mean this works without needing to add the PREFIX to the CMAKE_FIND_ROOT_PATH.

ReenigneCA commented 2 years ago

So to sum up where I'm at:

I forgot to take your updates to my previous commit sorry for all the little syntax things that seeped back in.

Some of the variable names and values I just maintained from the old version (some erroneously from the stable tarball,) I'll clean them up to the current values from master or in the case of variable names I'll change them when I get a thumbs up on the appropriate comment.

I mostly write code and actually really hate dealing with build systems and especially with shell scripts so I misunderstood the desired output format. The old format had the architecture embedded in the prefix itself so you couldn't easily have multiple architectures and multiple libraries in the same directory structure. As long as the arch is there this shouldn't be a problem. I'll switch to a ${PREFIX}/${ARCH}/include ${PREFIX}/${ARCH}/lib format and clean up the CMAKE recommendations. I'm pretty sure the only real difference CMAKE wise will be the need to specify the arch when setting the search root. I have to run for now I'll go through and resolve comments when I get a chance and alter the default structure after I get a chance to test the differences with an Android project.

jedisct1 commented 2 years ago

A pkg-config configuration is also installed, so maybe we can recommend using it instead.

This has several advantages:

I don't know much about cmake, but it seems to have out of the box support for png-config as well:

find_package(PkgConfig REQUIRED)
pkg_search_module(LIBSODIUM REQUIRED libsodium>=1.0.18)

target_compile_options(HelloWorld PUBLIC ${LIBSODIUM_STATIC_CFLAGS} ${LIBSODIUM_STATIC_CFLAGS_OTHER} ${LIBSODIUM_STATIC_LDFLAGS} ${LIBSODIUM_STATIC_LDFLAGS_OTHER})
target_include_directories(HelloWorld PUBLIC ${LIBSODIUM_STATIC_INCLUDE_DIRS})
target_link_directories(HelloWorld PUBLIC ${LIBSODIUM_STATIC_LIBRARY_DIRS})
target_link_libraries(HelloWorld PUBLIC ${LIBSODIUM_STATIC_LIBRARIES})

And the base directory just has to be specified in CMAKE_PREFIX_PATH.

Regarding the target API version, libsodium doesn't use much of the native Android system libraries, so targeting the lowest supported API may be fine. Unless an application designed for a different API won't be able to link a library compiled for a lower API? If that is the case, we may want to include the API level in the base directory name.

jedisct1 commented 2 years ago

There was also a subtle difference between TARGET_ARCH and the architecture as exposed in directory names.

The target architecture can include CPU feature flags (such as +crypto for armv8-a). The resulting library will work on the baseline CPU without these extensions, however, if these extensions are detected at compile-time, they will be enabled. And in order for the compiler to accept code using these extensions, they have to be enabled at compile-time.

This is what broke the ARM builds in that PR.

ReenigneCA commented 2 years ago

After all of my investigation into the best way to lay out libraries for the ndk I've come full circle. The way I was trying to lay things out is according to the standard the ndk uses for it's sysroot (I actually also discovered that you can make separate directories for different android versions it uses the layout that this pull request uses plus a directory that's just the android api number,) and if you put things in that format into the sysroot lib folder things compile using -lsodium. This seemed to me to be a very convenient way to set things up, but while programs will compile, android studio will not include the libs in a generated apk if they are in the sysroot, which in retrospect makes a lot of sense. Personally I like this format better still only because I want one lib folder with all my archs in it, but that is definitely outweighed by anything that depends on the current format and is something I can do with a python script on my end. So while I really thought I was learning things to make things better for everyone after everything I now see I was on the totally wrong track.

That being said this rabbit hole has raised a few questions about the current scripts

I think it makes sense to close this pull request without merging it but if you want to comment on / add to the above list I'll make a new branch (not from an old stable version this time I promise :P ) and try to cover everything off.

Edit: The NDK_PLATFORM variable is actually used to set the SDK_PLATFORM version... 19 right now covers 99.4% of devices while 21 covers 98% of devices according to the Android Studio new project wizard. The difference now makes sense to me though as 21 was the first platform that supported 64 bit processors. I'm not sure if there is any downside to the library to keep compiling for 19 for 32 bit devices I think the best approach would be to use an if statement to enforce a different default in android-build.sh so that higher level scripts can override the setting for all platforms in one place.