-
Notifications
You must be signed in to change notification settings - Fork 596
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
Firebase In-App Messaging conflits with Dagger | Don't use dagger | Vendor dagger #1677
Comments
I found a few problems with this issue:
|
This issue also impacts
Given how widely Firebase is used, it seems to me like this |
Unfortunately, the best we can help with on this front is to point you to ways to work around this situation. See: #1643 (comment) |
I can't tell what the 'conflict' you are seeing is. If you are referring to the same problem that @jhansche is referring to, the suggested workarounds are covered here #1677 (comment). Please reopen if that's not what you are referring to |
I don't think this is the right solution. See the linked Dagger comment thread, where Dagger's stance is made more clear (multiple times), that third party libraries should be using a shaded version of Dagger, so that their choices don't collide with anyone else's: |
If I'm understanding correctly, the issue @Alison-Soldado is describing is the same as what I described:
^ Their app depends on Dagger v2.17, but Firebase's transitive dependencies are causing an unexpected forced version bump during Gradle's dependency version conflict resolution, such that their 2.17 dependency automatically gets bumped to 2.27 out from under them and with no warning. That creates a compatibility issue, similar to those mentioned in google/dagger#950. It's also different from #1643 which is about Dagger1 vs Dagger2, and the main problem there is because of the Especially when the transitive dependency only forces one of the dependencies to get updated, while the others all remain on the older 2.17 version. |
@jhansche Libraries shipping with shaded dagger and vendoring a copy will optimize for consumers that are on the older versions of dagger. It would lead to customers who are on newer versions having to take on unnecessary footprint. That's the reason we are leaving the shadowing up to customers. Can you show what the challenges are migrating your code up to the new version? That would would help us prioritize this a little better. |
I think it's less about our own use of it, and more the fact that this unexpectedly influences the dependency graph of every app that uses it (and Firebase is very popular, so that includes a lot of apps). I agree that updating Dagger is usually painless, but an app can only do that if they know it is a requirement. What's worse, is Gradle will not complain about this scenario, and the code will compile, but it might crash at runtime. If every third party library operated like this, then multiple libraries may be using competing versions, and given that there are binary incompatibility issues that have come up in multiple versions of Dagger, that could mean that an app having 2 different libraries that each want a different version of Dagger (whether or not the app is able to update their version to match, which I do agree is normally a simple task) could be put in a situation that's impossible to resolve.
Actually, I would argue that it's optimizing for the case of all apps and libraries that reference this datatransport library to never have to worry about this problem again. That, in my opinion, far outweighs any arguments of "older vs newer." The Dagger runtime is relatively small (~33KB jar, 53KB uncompressed), but the runtime artifact is only guaranteed to work if the generated code was generated with the same version of the dagger compiler, as noted in the comments above from the Dagger team. When a library like this influences the dependency graph, it's only going to coerce the runtime artifact to match the library's runtime, but will not automatically bump the build-time compiler/spi/provider artifact. In the OP's case, that means their app will have code generated with the 2.17 Dagger compiler, while Firebase forces a bump of Dagger runtime to 2.27, and those are not compatible.
But the Dagger team (@ronshapiro) recommends the opposite 😞 |
Fair!
Thanks for raising. I did not realize that this would fail only at runtime. Is there a crash you can share ?
I'll share why this is a problem. Alternatively they could share a release cadence that allows them to share a copy. That's sub optimal for maintainability. Not to mention, it would force you to upgrade all or nothing, a world we have lived in previously.
We have a couple of options.
Any others come to mind ? |
My suggestion was actually related to datatransport having its Dagger version shaded. It does not look like Firebase directly requires anything from that Dagger dependency, so shading it inside datatransport would hide it not only from apps, but even hides it from Firebase itself. And then all Firebase SDKs that use datatransport would automatically resolve to a single version (per Gradle's resolution rules), and there's no need to negotiate the shaded-dagger version in Firebase.
I don't currently have one, but the scenario would occur the same way as the Dagger issue linked above, starting at: In that particular case, the MembersInjector class was removed as of Dagger v2.15, so any apps using a previous version, would have their code generated using the pre-2.15 compiler, and a transitive dependency would cause a bump to post-2.15. That bump means the MembersInjector class is no longer present in runtime, but the app's code was compiled with it present on the classpath. So no errors at compile time, but the app's Dagger code would crash at runtime because the class cannot be found. And a similar issue would happen if a library used an older version (so their generated code was generated with the older compiler), and the app is the one that requests an updated version. Same thing, just reversed, so the crash occurs in the library code, not the app code. |
There are libraries other than data transport that use dagger too. |
Hey @Alison-Soldado. We need more information to resolve this issue but there hasn't been an update in 5 weekdays. I'm marking the issue as stale and if there are no new updates in the next 5 days I will close it automatically. If you have more information that will help us get to the bottom of this, just add a comment! |
Pls ignore @google-oss-bot. No more information needed at this point. |
Causes an runtime exception static method membership injector cannot be found |
Hey Sanjeev, |
Yep, I have the PR up here: #1821 . But I got kind of stuck trying to get instrumentation tests to pass. I will try to make more progress on it in the coming days |
The issue is fixed in the latest release of fiam and fiamd |
Just letting you guys (and other people reading this) know, this may be fixed for |
@acrespo sorry about that, the next release of crashlytics will fix this, in the meantime you can manually include the following as a workaround:
|
Man, that was fast! Thanks! I managed to upgrade Dagger and everything went well but I'm glad to hear there was another option. Maybe it'll help someone else. Keep up the good work! |
This worked for me. I have excluded datatransport group and implemented com.google.firebase:firebase-datatransport:17.0.8 separtely and can able to run the application without runtime crash. Waiting for next release soon. |
[REQUIRED] Step 2: Describe your environment
[REQUIRED] Step 3: Describe the problem
Steps to reproduce:
After added the dependency in-app messaging cannot find symbol DaggerAppComponent
Relevant Code:
build.gradle(:app)
build.gradle
The text was updated successfully, but these errors were encountered: