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

[Android][WIP] Migrate to FBJNI to support Flipper upgrade #27729

Closed
wants to merge 20 commits into from

Conversation

@passy
Copy link
Member

passy commented Jan 10, 2020

Summary

This is an incomplete effort to migrate from libfb to libfbjni. This is needed to restore the compatibility with Flipper and other FB Android projects that make use of FBJNI. Effectively, the outcome is that fbjni no longer has a checked-in copy here, but instead relies on the public artifacts published at github.com/facebookincubator/fbjni that can be deduplicated at build-time.

A non-exhaustive list of tasks:

  • Gradle builds the SDK and RNTester for Android.
  • Buck build for rntester works in OSS.
  • Move from java-only release to full fbjni release. This requires finding a solution for stripping out .so files that the old Android.mk insists on including in the final artifacts and will clash with the full distribution.
  • Import this and fix potential internal build issues.
  • Verify that the changes made to the Hermes integration don't have any unintended consequences.

Changelog

[Android] [Changed] - Migrated from libfb to libfbjni for JNI calls

Test Plan

  • CI is already passing again for Gradle and Buck in OSS.
  • After applying the following patch, RNTester builds and works with the latest Flipper SDK:
diff --git a/RNTester/android/app/build.gradle b/RNTester/android/app/build.gradle
index b8a6437d7..eac942104 100644
--- a/RNTester/android/app/build.gradle
+++ b/RNTester/android/app/build.gradle
@@ -170,10 +170,19 @@ dependencies {
     debugImplementation files(hermesPath + "hermes-debug.aar")
     releaseImplementation files(hermesPath + "hermes-release.aar")
 
-    debugImplementation("com.facebook.flipper:flipper:0.23.4") {
+    debugImplementation("com.facebook.flipper:flipper:+") {
         exclude group:'com.facebook.yoga'
-        exclude group:'com.facebook.flipper', module: 'fbjni'
-        exclude group:'com.facebook.litho', module: 'litho-annotations'
+        exclude group:'com.facebook.fbjni'
+    }
+
+    debugImplementation("com.facebook.flipper:flipper-network-plugin:+") {
+        exclude group:'com.facebook.yoga'
+        exclude group:'com.facebook.fbjni'
+    }
+
+    debugImplementation("com.facebook.flipper:flipper-fresco-plugin:+") {
+        exclude group:'com.facebook.yoga'
+        exclude group:'com.facebook.fbjni'
     }
 
     if (useIntlJsc) {
passy added 20 commits Nov 27, 2019
Summary:
There was a reflective call to a non-existent class. It did, however,
exist in the template, so I copied it over from there and updated
the references accordingly.

Test Plan:
Built it and ran it. Works again with the latest Flipper desktop app.
@passy passy changed the title [WIP] Migrate to FBJNI [Android][WIP] Migrate to FBJNI Jan 10, 2020
Copy link

facebook-github-bot left a comment

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

Copy link
Collaborator

gengjiawen left a comment

good job.

@gengjiawen

This comment has been minimized.

Copy link
Collaborator

gengjiawen commented Jan 14, 2020

Looks like internal tests failed. cc @passy

@passy

This comment has been minimized.

Copy link
Member Author

passy commented Jan 14, 2020

@gengjiawen Yep, there's still a lot of internal build infra that needs to be adjusted before this can get landed.

@rickhanlonii rickhanlonii changed the title [Android][WIP] Migrate to FBJNI [Android][WIP] Migrate to FBJNI to support Flipper upgrade Jan 17, 2020
@facebook-github-bot

This comment has been minimized.

Copy link

facebook-github-bot commented Jan 20, 2020

This pull request was exported from Phabricator. Differential Revision: D19345270

passy added a commit to passy/react-native that referenced this pull request Jan 20, 2020
Summary:
This is an incomplete effort to migrate from libfb to libfbjni. This is needed to restore the compatibility with Flipper and other FB Android projects that make use of FBJNI. Effectively, the outcome is that `fbjni` no longer has a checked-in copy here, but instead relies on the public artifacts published at github.com/facebookincubator/fbjni that can be deduplicated at build-time.

**A non-exhaustive list of tasks:**

* [X] Gradle builds the SDK and RNTester for Android.
* [X] Buck build for rntester works in OSS.
* [ ] Move from `java-only` release to full `fbjni` release. This requires finding a solution for stripping out `.so` files that the old `Android.mk` insists on including in the final artifacts and will clash with the full distribution.
* [ ] Import this and fix potential internal build issues.
* [ ] Verify that the changes made to the Hermes integration don't have any unintended consequences.

## Changelog

[Android] [Changed] - Migrated from libfb to libfbjni for JNI calls
Pull Request resolved: facebook#27729

Test Plan:
- CI is already passing again for Gradle and Buck in OSS.
- After applying the following patch, RNTester builds and works with the latest Flipper SDK:

```
 diff --git a/RNTester/android/app/build.gradle b/RNTester/android/app/build.gradle
index b8a6437d7..eac942104 100644
 --- a/RNTester/android/app/build.gradle
+++ b/RNTester/android/app/build.gradle
@@ -170,10 +170,19 @@ dependencies {
     debugImplementation files(hermesPath + "hermes-debug.aar")
     releaseImplementation files(hermesPath + "hermes-release.aar")

-    debugImplementation("com.facebook.flipper🐬0.23.4") {
+    debugImplementation("com.facebook.flipper🐬+") {
         exclude group:'com.facebook.yoga'
-        exclude group:'com.facebook.flipper', module: 'fbjni'
-        exclude group:'com.facebook.litho', module: 'litho-annotations'
+        exclude group:'com.facebook.fbjni'
+    }
+
+    debugImplementation("com.facebook.flipper:flipper-network-plugin:+") {
+        exclude group:'com.facebook.yoga'
+        exclude group:'com.facebook.fbjni'
+    }
+
+    debugImplementation("com.facebook.flipper:flipper-fresco-plugin:+") {
+        exclude group:'com.facebook.yoga'
+        exclude group:'com.facebook.fbjni'
     }

     if (useIntlJsc) {
```

Reviewed By: mdvacca

Differential Revision: D19345270

Pulled By: passy

fbshipit-source-id: 2530c93d23d762ecde13bf7d94766fb7ba0df5e5
@facebook-github-bot

This comment has been minimized.

Copy link

facebook-github-bot commented Jan 20, 2020

This pull request was exported from Phabricator. Differential Revision: D19345270

passy added a commit to passy/react-native that referenced this pull request Jan 20, 2020
Summary:
This is an incomplete effort to migrate from libfb to libfbjni. This is needed to restore the compatibility with Flipper and other FB Android projects that make use of FBJNI. Effectively, the outcome is that `fbjni` no longer has a checked-in copy here, but instead relies on the public artifacts published at github.com/facebookincubator/fbjni that can be deduplicated at build-time.

**A non-exhaustive list of tasks:**

* [X] Gradle builds the SDK and RNTester for Android.
* [X] Buck build for rntester works in OSS.
* [ ] Move from `java-only` release to full `fbjni` release. This requires finding a solution for stripping out `.so` files that the old `Android.mk` insists on including in the final artifacts and will clash with the full distribution.
* [ ] Import this and fix potential internal build issues.
* [ ] Verify that the changes made to the Hermes integration don't have any unintended consequences.

## Changelog

[Android] [Changed] - Migrated from libfb to libfbjni for JNI calls
Pull Request resolved: facebook#27729

Test Plan:
- CI is already passing again for Gradle and Buck in OSS.
- After applying the following patch, RNTester builds and works with the latest Flipper SDK:

```
 diff --git a/RNTester/android/app/build.gradle b/RNTester/android/app/build.gradle
index b8a6437d7..eac942104 100644
 --- a/RNTester/android/app/build.gradle
+++ b/RNTester/android/app/build.gradle
@@ -170,10 +170,19 @@ dependencies {
     debugImplementation files(hermesPath + "hermes-debug.aar")
     releaseImplementation files(hermesPath + "hermes-release.aar")

-    debugImplementation("com.facebook.flipper🐬0.23.4") {
+    debugImplementation("com.facebook.flipper🐬+") {
         exclude group:'com.facebook.yoga'
-        exclude group:'com.facebook.flipper', module: 'fbjni'
-        exclude group:'com.facebook.litho', module: 'litho-annotations'
+        exclude group:'com.facebook.fbjni'
+    }
+
+    debugImplementation("com.facebook.flipper:flipper-network-plugin:+") {
+        exclude group:'com.facebook.yoga'
+        exclude group:'com.facebook.fbjni'
+    }
+
+    debugImplementation("com.facebook.flipper:flipper-fresco-plugin:+") {
+        exclude group:'com.facebook.yoga'
+        exclude group:'com.facebook.fbjni'
     }

     if (useIntlJsc) {
```

Reviewed By: mdvacca

Differential Revision: D19345270

Pulled By: passy

fbshipit-source-id: e61f94043b471f922f701973284f0ceabd1a05e1
@react-native-bot

This comment has been minimized.

Copy link
Collaborator

react-native-bot commented Jan 21, 2020

This pull request was successfully merged by @passy in 9ad5e72.

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

passy added a commit to passy/react-native that referenced this pull request Jan 21, 2020
Summary:
This fixes a build failure with buck introduced with facebook#27729. The internal and external buck overlays diverged in how fbjni was imported. The Buck re-export here ensures that the targets resolve both internall and externally.

## Changelog

[Android] [Fixed] - RNTester Buck Build
Pull Request resolved: facebook#27826

Test Plan:
buck fetch rntester
buck build rntester

Reviewed By: cpojer

Differential Revision: D19496769

Pulled By: passy

fbshipit-source-id: 9537c72b9d6fef14639eb7d79caec017cef2eb17
facebook-github-bot added a commit that referenced this pull request Jan 21, 2020
Summary:
This fixes a build failure with buck introduced with #27729. The internal and external buck overlays diverged in how fbjni was imported. The Buck re-export here ensures that the targets resolve both internall and externally.

## Changelog

[Android] [Fixed] - RNTester Buck Build
Pull Request resolved: #27826

Test Plan:
buck fetch rntester
buck build rntester

Reviewed By: jknoxville

Differential Revision: D19496769

Pulled By: passy

fbshipit-source-id: d699a5f64f691ed375cfc7a9d6a5a6f6e36ba283
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.