firebase / firebase-android-sdk

Firebase Android SDK
https://firebase.google.com
Apache License 2.0
2.26k stars 572 forks source link

[Crashlytics] cyclic dependency between gradle tasks becuase of injectCrashlyticsBuildIdsRelease depends on CMakeBuild #6167

Open mar3kk opened 1 month ago

mar3kk commented 1 month ago

I couldn't add comment to the already closed issue https://github.com/firebase/firebase-android-sdk/issues/5354 thus opened new one.

Original description

Environment Android Studio version: Iguana 2023.2.1 Firebase Component: Crashlytics Gradle Plugin Component version: 2.9.9 (But started in 2.9.5) The problem Cyclic dependency between gradle tasks after trying to upgrade Crashlytics Gradle Plugin to 2.9.9. In our setup we first build java/kotlin code, obfuscate it using R8, then use the R8 outputs to obfuscate JNI headers. This makes the buildCMake task depend on building java/kotlin code and R8.

After updating to 2.9.9 the injectCrashlyticsBuildIdsRelease task kicks in which seems to be necessary to mergeReleaseResources but at the same time depends on outputs from the native build which is the opposite order we have in our code. This causes cyclic dependency between gradle tasks. See output from the console:

:app:buildCMakeRelWithDebInfo[armeabi-v7a]
\--- :app:configureCMakeRelWithDebInfo[armeabi-v7a]
     \--- :libom_generated:generateReleaseJniObfuscation
          +--- :libom_generated:generateReleaseJniHeaders
          |    \--- :app:compileReleaseJavaWithJavac
          |         +--- :app:compileReleaseKotlin
          |         |    +--- :app:dataBindingGenBaseClassesRelease
          |         |    |    +--- :app:mergeReleaseResources
          |         |    |    |    \--- :app:injectCrashlyticsBuildIdsRelease
          |         |    |    |         \--- :app:stripReleaseDebugSymbols
          |         |    |    |              \--- :app:mergeReleaseNativeLibs
          |         |    |    |                   \--- :app:buildCMakeRelWithDebInfo[armeabi-v7a] (*)
          |         |    |    \--- :app:parseReleaseLocalResources
          |         |    |         \--- :app:packageReleaseResources
          |         |    |              \--- :app:injectCrashlyticsBuildIdsRelease (*)
          |         |    +--- :app:kaptReleaseKotlin
          |         |    |    \--- :app:kaptGenerateStubsReleaseKotlin
          |         |    |         +--- :app:dataBindingGenBaseClassesRelease (*)
          |         |    |         +--- :app:kspReleaseKotlin
          |         |    |         |    +--- :app:generateReleaseSources
          |         |    |         |    |    \--- :app:injectCrashlyticsBuildIdsRelease (*)
          |         |    |         |    \--- :app:processReleaseResources
          |         |    |         |         +--- :app:mapReleaseSourceSetPaths
          |         |    |         |         |    \--- :app:injectCrashlyticsBuildIdsRelease (*)
          |         |    |         |         +--- :app:mergeReleaseResources (*)
          |         |    |         |         \--- :app:parseReleaseLocalResources (*)
          |         |    |         \--- :app:processReleaseResources (*)
          |         |    +--- :app:kspReleaseKotlin (*)
          |         |    \--- :app:processReleaseResources (*)
          |         +--- :app:dataBindingGenBaseClassesRelease (*)
          |         +--- :app:generateReleaseSources (*)
          |         +--- :app:kaptReleaseKotlin (*)
          |         +--- :app:kspReleaseKotlin (*)
          |         \--- :app:processReleaseResources (*)
          \--- :libom_generated:generateReleaseProguardMapping
               \--- :app:compileReleaseJavaWithJavac (*)

In the mentioned issue https://github.com/firebase/firebase-android-sdk/issues/5354 there was a suggestion to generate the R8 mapping upfront and provide that mapping to R8 using the -applymapping rule. We started investigating this option. We used KSP to generate the preliminary mapping file. However we faced an issue where JNI headers are generated by the java compiler (javac -h). This still establish the task order where native code can only be compiled after javac/kotlinc.

We are currently investigating if we could utilize the KSP to generate the JNI headers as well. However I think you should really consider the idea of keeping the build ids in assets rather than as resources.

lehcar09 commented 1 month ago

Hi @mar3kk, thank you for reaching out again. I'll notify our engineers about this and see what we can here. Thanks!

mrober commented 1 month ago

I considered doing this before. The thing that stopped me is we didn't want to create a dependency requirement between versions of the Gradle plugin and versions of the SDK.

So instead, I can do this in the plugin behind a flag, and then require a gradle property and the latest SDK for it to work.

mar3kk commented 1 month ago

I think having extra flag for that is perfectly fine for us. We are also evaluating if we can get rid of javac to generate jni headers. Side question: We still use old version of the Gradle plugin, but we keep updating Crashlytics. Is there actually any benefit having latest plugin?

lehcar09 commented 1 month ago

We mark this as a feature request. While we are unable to promise any timeline for this, we'll definitely keep this under our radar.

P.S. For folks who find this useful, adding an emoji thumbs up on the original post can help us prioritize adding this to the roadmap.

As for your side question:Firebase Crashlytics Gradle plugin streamlines integrating Crashlytics into your Android project. It:

For optimal performance and stability, always use the latest Firebase SDK/plugin. This ensures you benefit from the most recent updates, bug fixes, and improvements.