Update Build.gradle #33507
Update Build.gradle #33507
Conversation
This change allows to configure the NDK version in root project.
|
Hi @Morritz! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
facebook-github-bot
added
the
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
| @@ -280,6 +280,7 @@ task androidSourcesJar(type: Jar) { | |||
|
|
|||
| android { | |||
| compileSdkVersion 31 | |||
| ndkVersion rootProject.ext.ndkVersion | |||
There was a problem hiding this comment.
I was setting this too earlier in Android Studio manually when integrating other libraries, since it wasn't being picked up from rootProject
So i've had a yarn patch stash sitting awhile locally, but avoided committing it in the end (could jus be me)
I don't see the harm including it, but assumed it wasn't for a reason - can't think of side-effects it would cause atm
There was a problem hiding this comment.
I think there needs to be such change made in order to make developer's life easier. Especially in early phases of development like starting with the template which already should have as I remember correctly some ndk version put there, but if developer wants to upgrade it to newer version manually then I see no problem there. Maybe it would be better for the ReactAndroid to update this ndk as internal dependency to support M1 platform, but still someone might want to use some version that he has already installed and if there is no problem with that as you mentioned, this change would be beneficial, because it would reduce digging through the deeps of the code to set it somewhere else.
There was a problem hiding this comment.
Mainly it helps with ndk-build, which was also addressed in other dev commit that i have seen in repo and that change was to replace ndk-build with cmake build.
There was a problem hiding this comment.
Gotcha and thanks the explanation earlier ^ appreciate it
Just back from a mini-break and spotted (thx too @cortinico)
- Should we release 0.68.1? reactwg/react-native-releases#19 (comment)
- Improve support for Android users on M1 machine #33588
Which looks like it'll resolve our NDK issue too, I'll check later today
@leotm you're right. I've pushed a change to allow customizing the NDK version on the top level, mostly for M1 users. The change is a bit more extensive than what was proposed by @Morritz as it needed to propagate the config for the I'm closing here, but thanks for the contrib @Morritz 🙏 |
This change allows to configure the NDK version in root project.
Summary
This change was motivated by the problem with compilation of new architecture on ARM chips with ndk-build.
This change combined with setting newer version inside root project's build.gradle allows end-user to compile successfully.
Changelog
[Android] [Feature] - This change allows user to change NDK version used by ReactAndroid inside their root project's build.gradle file.
Test Plan