facebookincubator / fbjni

A library designed to simplify the usage of the Java Native Interface
Apache License 2.0
266 stars 47 forks source link

fbjni-0.5.0-headers.jar responds in 404 #85

Closed stiltet-gofore closed 1 year ago

stiltet-gofore commented 1 year ago

Issue description

When deploying our React Native 0.70.6 app to Android through Fastlane we're now getting failed builds with the following error;

* Where:
Build file 'node_modules/react-native-mmkv/android/build.gradle' line: 322

* What went wrong:
Execution failed for task ':react-native-mmkv:extractAARHeaders'.
> Could not resolve all files for configuration ':react-native-mmkv:extractHeaders'.
   > Could not find fbjni-0.5.0-headers.jar (com.facebook.fbjni:fbjni:0.5.0).
     Searched in the following locations:
         https://repo.maven.apache.org/maven2/com/facebook/fbjni/fbjni/0.5.0/fbjni-0.5.0-headers.jar

Code example

When looking through https://repo.maven.apache.org/maven2/com/facebook/fbjni/fbjni we see that the headers file is missing only for version 0.5.0.

https://repo.maven.apache.org/maven2/com/facebook/fbjni/fbjni/0.5.0/fbjni-0.5.0-headers.jar responds in 404.

System Info

Deployment from Linux CI env. using Fastlane.

cortinico commented 1 year ago

Thanks for the report. The -headers artifact is effectively missing, I'll look into it.

At the same time, the fact that mmkv had a dependency on fbjni with version "+" is a huge problem for you @stiltet-gofore and other users https://github.com/mrousavy/react-native-mmkv/blob/8e9231c3588336fec18769ca2dcb23d4030eeaea/android/build.gradle#L157-L160

I can publish the -header artifact, and your build will be green, but your app may crash at runtime as FBJNI 0.5.0 has been built with NDK 25, which will most likely be higher than the NDK version you're using in the rest of your setup.

stiltet-gofore commented 1 year ago

Okay, thanks. I can report an issue in the React Native MMKV repo about this. What type of runtime crashes are you referring to @cortinico?

andreaSimonePorceddu commented 1 year ago

Same issue with react-native-quick-crypto

* Where:
Build file ‘*/node_modules/react-native-quick-crypto/android/build.gradle’ line: 337
* What went wrong:
Execution failed for task ‘:react-native-quick-crypto:extractAARHeaders’.
> Could not resolve all files for configuration ‘:react-native-quick-crypto:extractHeaders’.
   > Could not find fbjni-0.5.0-headers.jar (com.facebook.fbjni:fbjni:0.5.0).
     Searched in the following locations:
         https://repo.maven.apache.org/maven2/com/facebook/fbjni/fbjni/0.5.0/fbjni-0.5.0-headers.jar
cortinico commented 1 year ago

I can report an issue in the React Native MMKV repo about this. What type of runtime crashes are you referring to @cortinico?

You will have SIGSEGV or other native crashes caused by using a version of React Native which was compiled with NDK 23 (or earlier) and fbjni which was compiled with NDK 25.

Same issue with react-native-quick-crypto

@andreaSimonePorceddu

That library is also affected and you should report an issue for them.

I'll be publishing 0.5.1 in the next hours, which will "solve" this issues, but it won't prevent your apps from those runtime crashes. Also to clarify, your applications were already exposed to those crashes with fbjni 0.4.0 (and previous version) because of those libraries using a "+" dependency.

CoSNaYe commented 1 year ago

@cortinico deeply appreciate for your quick reply.

At the same time, the fact that mmkv had a dependency on fbjni with version "+" is a huge problem for you @stiltet-gofore and other users

About this, could you teach us how to fix the fbjni version to 0.4.0? maybe at least we could patch the library temporarily

https://github.com/mrousavy/react-native-mmkv/blob/8e9231c3588336fec18769ca2dcb23d4030eeaea/android/build.gradle#L157-L160

  extractHeaders("com.facebook.fbjni:fbjni:+:headers")
  extractJNI("com.facebook.fbjni:fbjni:+")
andreaSimonePorceddu commented 1 year ago

@cortinico We already reported the issue here https://github.com/margelo/react-native-quick-crypto/issues/185. But it seems strange that we have a breaking change that doesn't allow probably, thousand of apps to build, and it needs multiple libraries to release a new version to get it working again. I think we are missing backward-compatibility in some changes here.

cortinico commented 1 year ago

About this, could you teach us how to fix the fbjni version to 0.4.0? maybe at least we could patch the library temporarily

You can use mmkv version 2.6.0+ as, from what I can see, does have a "+" dependency.

About this, could you teach us how to fix the fbjni version to 0.4.0?

You can use patch-package to unblock this build failure and edit those two lines to:

  extractHeaders("com.facebook.fbjni:fbjni:0.4.0:headers")
  extractJNI("com.facebook.fbjni:fbjni:0.4.0")

but again, 0.4.0 might not be the right version you need.

Or, given that 0.5.0 sounds a breaking change. should this new version be 1.0.0?

This is not a breaking change. Also this library doens't follow semantic versioning as it's on a 0.x release schedule.

But it seems strange that we have a breaking change that doesn't allow probably, thousand of apps to build, and it needs multiple libraries to release a new version to get it working again. I think we are missing backward-compatibility in some changes here.

The core of the problem is that a library (mmkv, quick-crypto) is telling you to depend on "any" version of fbjni. So whenever you build, you'll get the latest version of fbjni available. As we update fbjni, we can offer compatibility with all the react native versions, just because mmkv or quick-crypto used a "+" dependency. It's up to that libraries to specify the version of their dependencies and don't use dynamic ranges.

antoine-wbr commented 1 year ago

Same with react-native-vision-camera.

CoSNaYe commented 1 year ago

thank you @cortinico

confirmed this worked for now 😅

diff --git a/node_modules/react-native-mmkv/android/build.gradle b/node_modules/react-native-mmkv/android/build.gradle
index 13eae76..d0dc001 100644
--- a/node_modules/react-native-mmkv/android/build.gradle
+++ b/node_modules/react-native-mmkv/android/build.gradle
@@ -155,9 +155,9 @@ dependencies {
   implementation 'com.facebook.react:react-native:+'

   //noinspection GradleDynamicVersion
-  extractHeaders("com.facebook.fbjni:fbjni:+:headers")
+  extractHeaders("com.facebook.fbjni:fbjni:0.4.0:headers")
   //noinspection GradleDynamicVersion
-  extractJNI("com.facebook.fbjni:fbjni:+")
+  extractJNI("com.facebook.fbjni:fbjni:0.4.0")

   if (!sourceBuild) {
     def buildType = "debug"

and about this:

@cortinico: You can use mmkv version 2.6.0+ as, from what I can see, does have a "+" dependency.

Because mmkv 2.6.0 has a breaking change:

Starting from 2.6.0., react-native-mmkv only works on react-native 0.71 and above. If you use react-native 0.70 and below, you should use react-native-mmkv 2.5.1.

so we couldn't upgrade the library unfortunately

zakus commented 1 year ago

The more global issue with those libraries (mmkv, quick-crypto) is that they try to use one version of headers, but react-native may use different version. In my case, for example, in ./node_modules/react-native/ReactAndroid/build.gradle react-native has

api("com.facebook.fbjni:fbjni-java-only:0.2.2")
extractHeaders("com.facebook.fbjni:fbjni:0.2.2:headers")
extractJNI("com.facebook.fbjni:fbjni:0.2.2")

But in ./node_modules/react-native-mmkv/android/build.gradle, the react-native-mmkv module. has:

extractHeaders("com.facebook.fbjni:fbjni:+:headers")
extractJNI("com.facebook.fbjni:fbjni:+")

i.e. the latest version, until today it was 0.4.0.

Hard-coding 0.4.0 version in the mmkv gradle file in this case would fix build, but indeed may lead to a crash, since headers do not correspond to the actual library used. The correct way is for mmkv to use exactly the same version as used by react-native.

But question is, how the proper fix for those libraries would look, how to get the current version of fbjni that is used by react-native, from the mmkv gradle script?

cortinico commented 1 year ago

Hard-coding 0.4.0 version in the mmkv gradle file in this case would fix build, but indeed may lead to a crash, since headers do not correspond to the actual library used. The correct way is for mmkv to use exactly the same version as used by react-native.

That's exactly what I was explaining in my previous comments @zakus thanks for stressing it again.

But question is, how the proper fix for those libraries would look, how to get the current version of fbjni that is used by react-native, from the mmkv gradle script?

There is no single fix for this, but it depends on a case by case basis:

cortinico commented 1 year ago

0.5.1 will be hitting Maven Central in the next minutes.

We'll follow up with more guidance on how to patch the affected libraries in the near future

andreaSimonePorceddu commented 1 year ago

This patch works for react-native-quick-crypto

diff --git a/node_modules/react-native-quick-crypto/android/build.gradle b/node_modules/react-native-quick-crypto/android/build.gradle
index 3ec9ff0..61cca86 100644
--- a/node_modules/react-native-quick-crypto/android/build.gradle
+++ b/node_modules/react-native-quick-crypto/android/build.gradle
@@ -172,9 +172,9 @@ dependencies {
   implementation 'com.facebook.react:react-native:+'

   //noinspection GradleDynamicVersion
-  extractHeaders("com.facebook.fbjni:fbjni:+:headers")
+  extractHeaders("com.facebook.fbjni:fbjni:0.5.1:headers")
   //noinspection GradleDynamicVersion
-  extractJNI("com.facebook.fbjni:fbjni:+")
+  extractJNI("com.facebook.fbjni:fbjni:0.5.1")

   if (!sourceBuild) {
     def buildType = "debug"
miradario commented 1 year ago

Adding these lines to react-native-vision-camera works for me. line: 292 of node_modules/react-native-vision-camera/android/buiild.gradle extractHeaders("com.facebook.fbjni:fbjni:0.5.1:headers") extractJNI("com.facebook.fbjni:fbjni:0.5.1")

stiltet-gofore commented 1 year ago

I can confirm that the patch-package worked for us.

mrousavy commented 1 year ago

Hey! I just published react-native-vision-camera 2.15.5 which fixes those issues!

In react-native-mmkv and react-native-quick-crypto, those issues are already fixed in the latest version :)

Thanks for giving me a head up about this!