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

Tidy up NDK 19+ support #1962

Closed
5 of 7 tasks
inclement opened this issue Aug 21, 2019 · 7 comments
Closed
5 of 7 tasks

Tidy up NDK 19+ support #1962

inclement opened this issue Aug 21, 2019 · 7 comments
Labels
ndk-r19+ pull requests or issues related with android ndk-r19 RELEASE BLOCKER

Comments

@inclement
Copy link
Member

inclement commented Aug 21, 2019

While merging #1722, #1944, #1947 (different aspects of NDK 19 support), I expect there may be some rough edges to smooth out. This issue exists to track things that need checking.

This is a release blocker, these things need checking before the next release.

@opacam
Copy link
Member

opacam commented Aug 22, 2019

@inclement, good idea, thanks for creating this issue!!! 😄

About the second point...regardingbuildozer, I think that this buildozer's PR will put us on the road. That PR is partially reviewed by @AndreMiras...probably it would be the best start point since it seems the less problematic...followed by an new buildozer release. What do you think?

@opacam opacam added the ndk-r19+ pull requests or issues related with android ndk-r19 label Aug 22, 2019
@inclement
Copy link
Member Author

inclement commented Aug 23, 2019

I think this requires more care than that, we want any buildozer user to get a very clear message about what to do, or for buildozer to handle things automatically. I'm more worried about users with existing installations than new ones.

Edit: But yes, merging this will be a first step.

@opacam
Copy link
Member

opacam commented Aug 24, 2019

I though that we had this covered via #1883 because we modify the proper values in #1722, any buildozer installation which runs p4a with an NDK older than 19 should have an error like:

Including testapp_pillow/main.py
Including testapp_pillow/colours.png
[INFO]:    Will compile for the following archs: arm64-v8a
[INFO]:    Getting Android API version from user argument: 27
[INFO]:    Available Android APIs are (14, 19, 20, 21, 24, 25, 26, 27, 28)
[INFO]:    Requested API target 27 is available, continuing.
[INFO]:    Getting NDK dir from from user argument
[INFO]:    Found NDK version 17c
[ERROR]:   Build failed: The minimum supported NDK version is 19. You can download it from https://developer.android.com/ndk/downloads/
[INFO]:    Instructions: Please, go to the android NDK page (https://developer.android.com/ndk/downloads/) and download a supported version.
*** The currently recommended NDK version is 19b ***

Process finished with exit code 1

I think that it's pretty clear, but this message doesn't cover buildozer, since it is a sister project, but maybe we could extend the p4a's message to cover it:

for buildozer users, first be sure to have an updated `buildozer` installation, then:
  - if you upgraded to buildozer <next release> or greater, the android's NDK download will be automatically downloaded...unless you modified one of the following keys from your `buildozer.spec` file: `android.ndk` or `android.ndk_path`. If that is the case, then go on with the reading...
  - uncomment and set the key: `android.ndk = <our recommended NDK dinamically set>`.
  - If you modified the key `android.ndk_path` of your `buildozer.spec` file, then manually download/install the recommended NDK and modify the paths accordingly.

...but we mix the projects...

Note: About NDK downloads in buildozer, I think that shouldn't be any problem, if buildozer handles 17c should handle 19 or 20 since nothing changed in google downloads (as far as I know).

@inclement
Copy link
Member Author

but maybe we could extend the p4a's message to cover it:

It would be preferable to have buildozer check the version and display the message. Buildozer users are not expected to need to read the p4a logs or handle the NDK download themselves.

The use cases that concern me are things like, a user could have a working buildozer/p4a setup using NDK 17c, which they probably don't want to update since it's a static working build target. If they then run buildozer in another directory, it will download the newest p4a and the build will fail. The user will be left unclear how to make buildozer work in both cases (and in fact they can't easily do so).

I think we probably want buildozer to do something like the following:

  • Check what NDK version is currently available (if any)
  • Check what p4a version is installed for the current project
  • If the p4a version is less than or equal to 2019.08.09, download NDK 17c (i.e. the recommended value) and log a warning with instructions on how to clean/update the NDK and p4a versions.
  • If the p4a version is greater than 2019.08.09, download NDK 19+ (i.e. the current recommended value)
  • If no p4a version is available, download p4a first then choose the NDK.

I haven't checked, but current issues might include:

  • Buildozer used to auto-update p4a whenever it could, we probably don't actually want it to do that (better to have a stable target anyway) but we do want a clear 'update p4a for me' command.
  • Buildozer probably tries to install the NDK well before it tries to install p4a, it needs to be able to do that afterwards and possibly have a new 'update the NDK for me' command.

@inclement
Copy link
Member Author

I should note as well: I can do the buildozer tests and any necessary code changes, I'm not asking you to do it unless you particularly want to.

@opacam
Copy link
Member

opacam commented Aug 24, 2019

Do it if you want and have time for it, I don't have any work done on that and I think that it will be better that I concentrate on all NDK r19 PRs related...but be aware that I think we have it partially covered...I think that to check for p4a's version is wrong, because we have implemented this one recently...so if we check for version we could get unexpected errors whenever we change NDK on p4a's develop branch...and I think that this mentioned buildozer feature it can be useful for us 😉

@inclement
Copy link
Member Author

Opened kivy/buildozer#961 for buildozer, but it doesn't actually change the behaviour as it does look like all the cases I was worried about are covered.

@misl6 misl6 closed this as completed Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ndk-r19+ pull requests or issues related with android ndk-r19 RELEASE BLOCKER
Projects
None yet
Development

No branches or pull requests

3 participants