NativeScript / android

NativeScript for Android using v8
https://docs.nativescript.org/guide/android-marshalling
Apache License 2.0
523 stars 134 forks source link

Specify ndkVersion for Android Runtime #1670

Closed FestplattenSchnitzel closed 3 years ago

FestplattenSchnitzel commented 3 years ago

Create a meaningful title

Specify ndkVersion for Android Runtime

Description

Does your commit message include the wording below to reference a specific issue in this repo?

Related Pull Requests

Does your pull request have unit tests?

cla-bot[bot] commented 3 years ago

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla. CLA has not been signed by users: @FestplattenSchnitzel. After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

FestplattenSchnitzel commented 3 years ago

This fixes a problem building the {N} Android Runtime at F-Droid. See https://gitlab.com/fdroid/fdroiddata/-/issues/2433.

FestplattenSchnitzel commented 3 years ago

Is it possible to get this merged without signing the CLA? I'd like to avoid exposing my real name to Google/NativeScript.

If not, I'd be glad if someone who already signed the CLA would recreate this PR.

farfromrefug commented 3 years ago

@FestplattenSchnitzel i agree 100% as mine is already exposed i ll create a new PR for you (tonight or monday)

farfromrefug commented 3 years ago

@FestplattenSchnitzel i am about to look at this. Could you explain the fixed version? What you want is to be able to fix the version right? I would prefer we give you a way to fix it . The thing is an app should be able to force that version directly from its app.gradle inside the android section. Now we could also add something like this:

def computeNdkVersion = { -> project.hasProperty("ndkVersion") ? ndkVersion :"21.4.7075529" }
ndkVersion computeNdkVersion()

21.4.7075529 being the default version for gradle 4.2 (https://developer.android.com/studio/projects/install-ndk#apply-specific-version)

Just like it is done for other props https://github.com/NativeScript/android-runtime/blob/master/test-app/app/build.gradle#L264

FestplattenSchnitzel commented 3 years ago

Could you explain the fixed version?

The F-Droid Buildserver currently fails to build the {N} Android runtime for an app, since it uses the wrong NDK version (22.1.7171670) and we don't know why. The only fix known so far and worked in the past is to specify the NDK version in the *.gradle file.

See also: GitLab issue.

The thing is an app should be able to force that version directly from its app.gradle inside the android section.

We are building the {N} Android runtime separately so this wouldn't help …

Now we could also add something like this:

Looks good.

FestplattenSchnitzel commented 3 years ago

Thank you for your efforts!

farfromrefug commented 3 years ago

@FestplattenSchnitzel one last thing before i do the PR. You know you can pass project properties to gradle through the command line https://kb.novaordis.com/index.php/Gradle_Variables_and_Properties#Project_Properties ? It means that if i did that:

def computeNdkVersion = { -> project.hasProperty("ndkVersion") ? ndkVersion :"21.4.7075529" }
ndkVersion computeNdkVersion()

you could pass the ndkVersion through the command line. The only think which bugs me is that i would prefer for ndkVersion to be "undefined" if it was not set throught project that way it would let gradle decide what to do (best in most cases).

FestplattenSchnitzel commented 3 years ago

You know you can pass project properties to gradle through the command line

Yes, I know about it, but haven't really used it yet.

In the build script we are doing gradle at the root of NativeScript/android-runtime at the moment. How could we specify the android.ndkVersion only for the runtime project via the properties?

farfromrefug commented 3 years ago

@FestplattenSchnitzel done https://github.com/NativeScript/android-runtime/pull/1671 you can close this PR

vishnuraghavb commented 3 years ago

Thank you so much for your support @farfromrefug! 😊