fix: react android kotlin plugin version conflict #34255

hurali97 · 2022-07-23T13:44:06Z

Summary

Basically, the react android conflicts with the kotlin version that's defined in the top level build.gradle. To resolve it, the approach was to get the kotlinVersion defined in top level build.gradle and use it. If it's not defined, we use the default(1.6.10 as of now).

The reason behind not using the DSL is that it doesn't allow us to use the variables due to it's constrained syntax.
See here

So the idea was to use the build script to resolve the kotlin plugin and it works 👍 .

Kindly asking for review @cortinico :)

Changelog

[Android] [Changed] - refactored usage of kotlin plugin

Test Plan

Ran the node ./scripts/run-ci-e2e-tests.js --js --android --ios to verify it doesn't introduce any unexpected issues.

analysis-bot · 2022-07-23T15:11:52Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 4899f8c
Branch: main

JB-CHAUVIN · 2022-07-25T15:58:01Z

LGTM, working on my big project.

Raden-Hor

LGTM

cortinico

Thanks for sending this over @hurali97 🙏 really appreciate.
Are we able to iterate on this a bit?

cortinico · 2022-07-29T16:14:11Z

ReactAndroid/build.gradle

+buildscript {
+    dependencies {
+        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:${rootProject.hasProperty("kotlinVersion") ? rootProject.ext.kotlinVersion : KOTLIN_VERSION}"
+    }
+}


I think this will have unwanted side effects. Specifically mixing buildscript{} and plugins{} in Gradle is not advised.

It seems like a workaround that we could live with 🤔 But I would look for a solution that uses only plugins{} if possible.

As far as I looked, having dynamic version in plugins isn't quite possible. Since, we require to use the kotlinVersion from root level if that's present.

See here

Can you add a note on why this is needed otherwise we'll forget about it

So the reason to go with this approach is:

use the kotlinVersion defined in top level build.gradle. If it's not defined, we use the default(1.6.10 as of now) defined in gradle.properties. We couldn't add dynamic version using the plugins DSL because of the constrained syntax, so we had to introduce the buildScript for this. See here

Should I add it as a comment in the build.gradle or are we good here?

We're good here 👍
I'm looking into cleaning this up a bit in the future so this will hopefully go.

cortinico · 2022-07-29T16:14:57Z

Plus can we make the CI green?

hurali97 · 2022-07-31T11:40:07Z

Plus can we make the CI green?

So CI is failing at:

/root/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java:19: error: package com.facebook.react.common.mapbuffer does not exist

and

/app/sdks/hermes/API/hermes/hermes.cpp:727:46: error: only virtual member functions can be marked 'override'

It would be great if you can help me out in this, as I don't think this might be related to the changes in this PR.

cortinico · 2022-07-31T12:09:27Z

/root/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java:19: error: package com.facebook.react.common.mapbuffer does not exist

This is actually related. com.facebook.react.common.mapbuffer is a package that contains only Kotlin code. So it means that the Kotlin plugin is not applied correctly and that code doesn't get compiled

hurali97 · 2022-07-31T12:46:23Z

/root/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java:19: error: package com.facebook.react.common.mapbuffer does not exist

This is actually related. com.facebook.react.common.mapbuffer is a package that contains only Kotlin code. So it means that the Kotlin plugin is not applied correctly and that code doesn't get compiled

Whoa, okay. I will update once I test it on local.

hurali97 · 2022-07-31T13:46:40Z

@cortinico I updated the build.gradle and kotlin compilation issue is resolved. But test_android on CI is failing because of the following:

/root/react-native/sdks/hermes/API/hermes/hermes.cpp:727:46: error: only virtual member functions can be marked 'override'
jsi::BigInt createBigIntFromInt64(int64_t) override;

Also build_hermes_macos fails because:

./utils/build-apple-framework.sh: line 91: cd: ./destroot/Library/Frameworks: No such file or directory

Any leads here?

cortinico · 2022-08-05T14:15:47Z

Any leads here?

IMHO a you should be find with a rebase this time

analysis-bot · 2022-08-05T16:20:15Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,610,467	+0
android	hermes	armeabi-v7a	7,026,972	+0
android	hermes	x86	7,911,133	+0
android	hermes	x86_64	7,884,455	+0
android	jsc	arm64-v8a	9,488,774	+0
android	jsc	armeabi-v7a	8,267,524	+0
android	jsc	x86	9,426,974	+0
android	jsc	x86_64	10,019,738	+0

Base commit: 051a677
Branch: main

hurali97 · 2022-08-05T17:59:16Z

@cortinico Thanks for your help 🙂 All is 🟢

facebook-github-bot · 2022-08-05T18:37:52Z

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

react-native-bot · 2022-08-08T15:10:04Z

This pull request was successfully merged by @hurali97 in be35c6d.

^{When will my fix make it into a release? | Upcoming Releases}

Summary: Fixes #34229 Basically, the react android conflicts with the kotlin version that's defined in the top level build.gradle. To resolve it, the approach was to get the `kotlinVersion` defined in top level build.gradle and use it. If it's not defined, we use the default(1.6.10 as of now). The reason behind not using the DSL is that it doesn't allow us to use the variables due to it's constrained syntax. See [here](https://docs.gradle.org/current/userguide/plugins.html#sec:constrained_syntax) So the idea was to use the build script to resolve the kotlin plugin and it works 👍 . Kindly asking for review cortinico :) ## Changelog  [Android] [Changed] - refactored usage of kotlin plugin Pull Request resolved: #34255 Test Plan: Ran the node ./scripts/run-ci-e2e-tests.js --js --android --ios to verify it doesn't introduce any unexpected issues. Reviewed By: dmitryrykun Differential Revision: D38468567 Pulled By: cortinico fbshipit-source-id: f9ab635fcf033f1d337ed90793ba1667957b8e01

Summary: Fixes facebook#34229 Basically, the react android conflicts with the kotlin version that's defined in the top level build.gradle. To resolve it, the approach was to get the `kotlinVersion` defined in top level build.gradle and use it. If it's not defined, we use the default(1.6.10 as of now). The reason behind not using the DSL is that it doesn't allow us to use the variables due to it's constrained syntax. See [here](https://docs.gradle.org/current/userguide/plugins.html#sec:constrained_syntax) So the idea was to use the build script to resolve the kotlin plugin and it works 👍 . Kindly asking for review cortinico :) ## Changelog  [Android] [Changed] - refactored usage of kotlin plugin Pull Request resolved: facebook#34255 Test Plan: Ran the node ./scripts/run-ci-e2e-tests.js --js --android --ios to verify it doesn't introduce any unexpected issues. Reviewed By: dmitryrykun Differential Revision: D38468567 Pulled By: cortinico fbshipit-source-id: f9ab635fcf033f1d337ed90793ba1667957b8e01

yakubbaev · 2024-03-10T12:17:52Z

Included in v0.70.0-rc.3

hurali97 added 2 commits July 23, 2022 18:30

chore: added kotlin version to gradle.properties

62e65ed

fix: moved kotlin plugin to build script to use kotlin version

5dee391

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 23, 2022

react-native-bot added the Platform: Android Android applications. label Jul 23, 2022

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jul 23, 2022

kelset requested a review from cortinico July 26, 2022 11:17

Raden-Hor reviewed Jul 27, 2022

View reviewed changes

cortinico reviewed Jul 29, 2022

View reviewed changes

fix: added apply kotlin plugin

94dd5b1

Merge branch 'facebook:main' into react-android/kotlin-plugin

b781cc2

facebook-github-bot closed this in be35c6d Aug 8, 2022

react-native-bot added the Merged This PR has been merged. label Aug 8, 2022

snyk-bot mentioned this pull request Sep 27, 2022

[Snyk] Upgrade react-native from 0.63.3 to 0.70.0 NOUIY/react-native-macos#752

Open

cipolleschi mentioned this pull request Nov 1, 2023

[RN][CI]Update node installation on debian (0.70) #41278

Closed

cipolleschi mentioned this pull request Jan 8, 2024

[0.70][RN][General] Migrate boost download url away from JFrog #42178

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: react android kotlin plugin version conflict #34255

fix: react android kotlin plugin version conflict #34255

hurali97 commented Jul 23, 2022 •

edited

Loading

analysis-bot commented Jul 23, 2022 •

edited

Loading

JB-CHAUVIN commented Jul 25, 2022

Raden-Hor left a comment

cortinico left a comment

cortinico Jul 29, 2022

hurali97 Jul 31, 2022

cortinico Aug 5, 2022

hurali97 Aug 5, 2022

cortinico Aug 5, 2022

cortinico commented Jul 29, 2022

hurali97 commented Jul 31, 2022

cortinico commented Jul 31, 2022

hurali97 commented Jul 31, 2022 •

edited

Loading

hurali97 commented Jul 31, 2022 •

edited

Loading

cortinico commented Aug 5, 2022

analysis-bot commented Aug 5, 2022

hurali97 commented Aug 5, 2022

facebook-github-bot commented Aug 5, 2022

react-native-bot commented Aug 8, 2022

yakubbaev commented Mar 10, 2024

fix: react android kotlin plugin version conflict #34255

fix: react android kotlin plugin version conflict #34255

Conversation

hurali97 commented Jul 23, 2022 • edited Loading

Summary

Changelog

Test Plan

analysis-bot commented Jul 23, 2022 • edited Loading

JB-CHAUVIN commented Jul 25, 2022

Raden-Hor left a comment

Choose a reason for hiding this comment

cortinico left a comment

Choose a reason for hiding this comment

cortinico Jul 29, 2022

Choose a reason for hiding this comment

hurali97 Jul 31, 2022

Choose a reason for hiding this comment

cortinico Aug 5, 2022

Choose a reason for hiding this comment

hurali97 Aug 5, 2022

Choose a reason for hiding this comment

cortinico Aug 5, 2022

Choose a reason for hiding this comment

cortinico commented Jul 29, 2022

hurali97 commented Jul 31, 2022

cortinico commented Jul 31, 2022

hurali97 commented Jul 31, 2022 • edited Loading

hurali97 commented Jul 31, 2022 • edited Loading

cortinico commented Aug 5, 2022

analysis-bot commented Aug 5, 2022

hurali97 commented Aug 5, 2022

facebook-github-bot commented Aug 5, 2022

react-native-bot commented Aug 8, 2022

yakubbaev commented Mar 10, 2024

hurali97 commented Jul 23, 2022 •

edited

Loading

analysis-bot commented Jul 23, 2022 •

edited

Loading

hurali97 commented Jul 31, 2022 •

edited

Loading

hurali97 commented Jul 31, 2022 •

edited

Loading