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

Issue with Cross-Compiling for Android on macOS Using libsodium-1.20.0-stable #1442

Open
restarajat opened this issue Jan 29, 2025 · 12 comments

Comments

@restarajat
Copy link

The minSdkVersion and targetSdkVersion defined in the libsodium-1.0.20.0.aar AndroidManifest.xml file are outdated.

Expected values (as per latest Android update):

minSdkVersion = 24
targetSdkVersion = 34
Current values in libsodium-1.0.20.0.aar/AndroidManifest.xml:

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="21" />

This may cause compatibility issues with newer Android versions, and it should be updated accordingly.


Question Regarding android-aar.sh

In android-aar.sh, I noticed this condition:

if [ $arch = "arm64-v8a" ] || [ $arch = "x86_64" ]; then
  SDK_VERSION="21"
else
  SDK_VERSION="19"
fi

Given that the minimum SDK version should be 21 for all architectures, why is it being set to 19 for certain architectures?
Is there any specific reason for maintaining a lower SDK version for those architectures? Would it be possible to update this logic to ensure consistency across all builds?

Guys, are you planning any new updates related to the version compatibility issues?

Originally posted by @restarajat in #1440

@jedisct1
Copy link
Owner

/cc @ReenigneCA

@jedisct1
Copy link
Owner

jedisct1 commented Jan 29, 2025

I don't know anything about Android, and the author of that script doesn't seem to be active on GitHub any more.

So I don't know what the implications of changing these values are.

Does it work if both android:minSdkVersion and android:targetSdkVersion are set to the same value as the NDK_PLATFORM version (21 by default)?

@ReenigneCA
Copy link
Contributor

I was never very active on GitHub. I have my own self hosted repo on my lan for side projects, my day job doesn't involve code and I have a toddler so I probably look that way but my account is active :P

The scripts were designed to support as many android versions as possible. So I think the sdk versions are changed to the minimum version that works for a given architecture either that or I just used the same values as the scripts I replaced. I won't have a chance to look closer today but I can try to get to it by the end of the week. I don't remember specifically why AMD64 and arm8 are special it would take me a while to track down the docs to figure that out.

@restarajat Do the AARs not work for you? Are you having trouble building then? The min SDK version and target versions shouldn't have much effect on the binaries produced if any at all. Setting them higher would prevent things designed for those older SDKs from working but quite likely won't affect newer versions at all particularly because libsodium doesn't depend on any android specific stuff.

I won't have time to look any closer today maybe by the end of the week. Generally Android recommended SDK versions have to do with supporting a reasonable number of in use devices based on google's analytics of how many active users there are on each version of Android. I am generally of the opinion that this is irrelevant to libsodium but if I can find some time I'll try to figure out why I have that if statement and if someone has trouble building or using the AAR then I'm happy to investigate and update the scripts.

@ReenigneCA
Copy link
Contributor

ChatGPT just got back to me and confirmed what I assumed so I think I've actually finished very early ala Scotty :P

I wasn't sure because SDK NDK and Android version numbers don't line up but as I suspected ARM8 and AMD64 weren't added until SDK version 21 so that is why the if statement is in there.

So SDK version 19 is when the cross compiler approach in the NDK was changed so the scripts themselves require version 19 as a min and prior to 21 AMD64 and ARM8 weren't supported so for every architecture the minimum architecture is set to the absolute minimum that can be supported.

So given libsodium is a C library that is not dependant on Android I believe it makes the most sense to leave things the way they are unless someone encounters build or usage issues with a newer version of android. If someone really wants to change those values it's pretty easy for them to do it themselves and the only issue that came to me prior to this was someone who was trying to build for an older platform vice a newer one.

@jedisct1
Copy link
Owner

Given that the current stable NDK only supports API >= 21 (headers and libraries for version 19 are not part of the package any more), wouldn't it still make sense to use 21 everywhere now?

@ReenigneCA
Copy link
Contributor

Sorry I was at a meeting. I haven't used the ndk in a while, didn't notice that change till you mentioned it. I'd say it depends on how the newest ndk handles the scripts. If it just silently uses 21 anyway I'd say we just leave it as is so that it works for people using older ndks with legacy development or add a conditional to bump it to 21 if the ndk is too new to support 19. I'm not really sure what strange old android machines might be floating around that might care about the change so bumping to 21 is probably fine as long as there is an actual benefit to bumping it. I'll do some tests when I get the chance and put in a PR if it seems to make sense to me.

@ReenigneCA
Copy link
Contributor

OK I've done some experimentation. Looks like there are already changes to the build scripts since I last saw that fix compile issues (the scripts in stable don't produce binaries for x86 or arm7 with an ndk newer than 25 but the main branch does,) but don't set the associated variables properly in the AAR. This likely doesn't matter at all but the AAR says that version 19 is supported for x86 and arm7 when in fact the SDK version is 21 as the build script changes the default (since no Android specific functionality is in use I would guess that the binaries likely do support 19 anyway but it isn't technically correct).

So the android-aar.sh script can be changed to just put 21 into the AAR metadata so it is correct or I can update the scripts to use 19 if the ndk is 25 or older (I checked and 26 also doesn't support SDK-19) and create correct AARs that support 19 when possible and 21 otherwise. I've already got the scripts that choose based on ndk version written I'd just have to clean them up and put in a PR. The other change wouldn't take any time at all just removing some lines and changing a couple of constants from 19 to 21.

@jedisct1 let me know which approach you'd prefer for the codebase and I'll get to work on a PR when I get a chance. I've got some other stuff on my plate for the afternoon but I should be able to get whichever you choose cleaned up and submitted tomorrow.

@jedisct1
Copy link
Owner

We can probably use the current LTS NDK as the base line. Setting everything to API level 21 feels reasonable and would be simpler than introduce logic for older NDKs.

@restarajat
Copy link
Author

While setting everything to API level 21 is technically possible, using API 24 as the baseline would be a more future-proof and optimized choice. It aligns with Google’s modern Android policies, ensures better security and performance, and avoids compatibility issues with third-party NDK libraries that have dropped support for API 21-23. Given the long-term trends, API 24+ is the safest and most practical option for maintaining stability and Play Store compliance.

@jedisct1
Copy link
Owner

using API 24 as the baseline would be a more future-proof and optimized choice

This is unlikely, because libsodium doesn't use the Android APIs.

Bumping the number to 24 would just make the AAR incompatible with a version supported by the current NDK.

@ReenigneCA
Copy link
Contributor

ReenigneCA commented Jan 31, 2025

@jedisct1 Additionally in cleaning everything up, since there won't be any difference between the architectures anymore I will make it so the value is easy to override by setting an environment variable in case someone wants to do something special or the ndk drops 21 support and someone wants to build an AAR before changes happen to the repo. I thought about eliminating the arch specific build scripts but I think they provide a convenient way in case someone wants to alter compile settings for a particular arch like target a newer sub-architecture or something.

@restarajat if you are still concerned about this go ahead and build AARs for 21 and 24 then compare the .so files using objdump and I think you'll find they'll come out identical (they will likely also be identical in binary form. I haven't confirmed this I'm confident enough it doesn't matter that I'm not going to bother). I suppose there could be some build flags that have changed defaults for the ndk compiler for different SDK levels but I highly doubt it. Anyway whatever changes could even in theory be there should absolutely not affect anything meaningful to an actual android app. Android uses a JVM environment on top of a linux kernel. The NDK is to write compiled code that interfaces with the linux kernel and other normal POSIX OS stuff. The SDK is to interface with the JVM layer, so as long as the ndk will compile the code then the build scripts are compatible with the NDK. Android Apps using the JVM environment link to and use libsodium not the other way around. libsodium is not built in any particular way to enable this communication it's done using a java foreign function interface approach built to run any library with C ABI compatibility. So unless I'm missing something huge the value really is basically meaningless as long as the code will compile and if it were meaningful we'd see bug reports of things not working.
Edit: Just to be a bit more accurate. The different sdk versions use different ABI settings for the kernel associated with that SDK version. That's why the value is used at all, but unless there are breaking ABI changes there is backwards compatibility and if this compatibility breaks it will be noticeable and things won't work. As long as things work it's the same compiler with the same flags just targeting a previous ABI which may or may not have changed at all and is likely to be compatible. If you go into your ndk you'll see that all the cross compilers are the same version of clang, in fact they're all shell scripts that call one copy of clang.

@ReenigneCA
Copy link
Contributor

ReenigneCA commented Jan 31, 2025

PR 1443 submitted. There are some shellcheck complaints on android-build.sh but they related to lines that were already there and they don't seem worth it to mess with at the moment.

@restarajat if you want SDK version 24 just run the new scripts with NDK_PLATFORM="android-28" ./android-aar.sh and you'll get 24 or whatever other version you want.

@jedisct1 if anything Android pops up in the future just @ me and I'll take a look when I get a chance. I don't routinely read issues or monitor the project but I like the opportunity to be involved and help.

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

3 participants