search

libpd / pd-for-android

Pure Data for Android
353 stars 91 forks source link

fix the build #113

Closed joebowbeer closed 3 years ago

joebowbeer commented 3 years ago

Fixing the build before I tackle the publish changes...

The build problem was because gradle could not resolve the ndk-build path.

I believe the root cause of failure was that getNdkBuildExecutionPath() was being evaluated too early. However, the builds still succeeded for a while because ANDROID_NDK_HOME was provided by GitHub Action's builder. Then, when AGP stopped reading these deprecated env vars, our builds failed.

Note that GitHub Action's builder image has the LTS version baked-in:

https://github.com/actions/virtual-environments/blob/main/images/linux/scripts/installers/android.sh

This is the ndkVersion we specify, saving us from having to install it ourselves.

Related:

https://issuetracker.google.com/issues/144111441 https://github.com/gradle/gradle/issues/12440 https://stackoverflow.com/questions/60404457

tkirshboim commented 3 years ago

When I tried to build this branch I received the following error:

Build file '../pd-for-android/PdCore/build.gradle' line: 89

A problem occurred evaluating project ':PdCore'.
> Could not create task ':PdCore:buildNative'.
   > NDK is not installed

The error message is misleading because an NDK installation was in place. The problem was not that NDK was not installed, but rather that a certain NDK version was not installed. I resolved this issue by installing NDK version 21.4.7075529. Is there a way that we could provide a better error message in this case? Maybe NDK version 21.4.7075529 is not installed?

If a specific NDK version is required to build, we should probably document this in the build instructions part of the Readme.

joebowbeer commented 3 years ago

I agree that this is confusing, and I hope it will improve.

The error message you receive is not generated by our code. (I think it is from NdkHandler.) The error message is generated when our code resolves android.ndkDirectory. Checking for null would fail in the same way; by the time our code checks that ndk-build is executable, the script has already failed...

Specifying the ndkVersion as we do is the recommended approach:

https://developer.android.com/studio/projects/configure-agp-ndk

This particular (LTS) version is specified because it is preinstalled in the GitHub builder image, but any 21.x version should work. However, this is not a new problem. I just changed the ndkVersion that is specified; if you did not already have the previous version installed then it would have failed before in the same way. It did for me, and also failed in CI.

I think the best opportunity for better messaging is to add something to the README. I'll update that after the publish changes are made.

joebowbeer commented 3 years ago

@tkirshboim I removed the unused defines.