-
Notifications
You must be signed in to change notification settings - Fork 523
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
Add ability to get p4a's recommended android's NDK version #947
Conversation
I haven't deeply checked yet, but that sounds like a very cool idea 👏 |
Oh yes, I will try to do something to cover this 😉 |
f5475c1
to
f668b11
Compare
Ok, added unittests, |
f668b11
to
f4e9eac
Compare
Awesome, thanks for that! I'll try reviewing later. I still have a lot to catch up with on other PR, but I have to say PR with tests usually get higher prio for me 😄 So you may have yours reviewed earlier than expected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I've installed the buildozer version of your fork.
pip3 install --upgrade--user /~https://github.com/opacam/buildozer/archive/feature-use-p4a-ndk.zip
And I made a build with p4a.branch = develop
.
Logs gave:
# Recommended android's NDK version by p4a is: 17c
Which is the ndk I'm using and build succeeded
buildozer/targets/android.py
Outdated
# recommended android's NDK version, otherwise return buildozer's one | ||
ndk_version = ANDROID_NDK_VERSION | ||
rec_file = join(pa_dir, "pythonforandroid", "recommendations.py") | ||
if (not os.path.exists(pa_dir)) or (not os.path.isfile(rec_file)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: why not only checking for os.path.isfile(rec_file)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AndreMiras 😄
Only if the p4a version used has the file `pythonforandroid/recommendations.py`, which contains the info we want, otherwise we will return the buildozer's one
f4e9eac
to
8dd22bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still looks good to merge to me
thanks @AndreMiras 😄 since it seems that @inclement has no opposition on this one (kivy/python-for-android#1962) I merge this 👍 |
So this way, if an user has not specified the android's NDK version via buildozer.spec's file, We will try to get the recommended one by p4a, established in
pythonforandroid.recommendations.py
file, so depending on the version of p4a used, we can use an android NDK or another.This will allow us to solve the problem where p4a needs to move to a newer android NDK in develop branch, but also we need to preserve the old one for p4a's master branch (like the
ndk-r19-migration
thing that we are dealing this days in p4a, kivy/python-for-android#1722).Note: in case that an old version of p4a is used, previous to 2019, where the p4a's recommendations file still not existed, We will fallback to buildozer's hardcoded NDK version