lightsail-network / java-stellar-sdk

The Java Stellar SDK library provides APIs to build transactions and connect to Horizon and Soroban-RPC Server.
https://javadoc.io/doc/network.lightsail/stellar-sdk
Apache License 2.0
175 stars 160 forks source link

Issues using the `Android Archive Library` hosted at jitpack.io #156

Closed dabitdev closed 1 year ago

dabitdev commented 5 years ago

Currently the AAR hosted at https://jitpack.io includes a local copy of all dependencies. The dependencies should be defined as any other maven library using transitive dependencies.

Now, for example I am working with retrofit / okhttp 3.12.0 and I am having plenty of clashes and errors. It is impossible to either debug looking at the dependency graph or excluding the dependencies from stellar library. Also it is not straight forward to see which okhttp is bundled inside.

On the other hand, to be able to use the library from jitpack I am forced to disable transitive dependencies since the library is clashing with himself.

  implementation("com.github.stellar:java-stellar-sdk:$stellarVersion") {
        transitive = false
    }
bartekn commented 5 years ago

Does #132 released in version 0.4.0 fix it for you?

dabitdev commented 5 years ago

I am using 0.3.3 and the issue is still there.

bartekn commented 5 years ago

Can you update to 0.4.0 and check again?

dabitdev commented 5 years ago

I just updated the library to 0.4.0, looks like the previous problem is solved. now, I can see that you are hosting the library okhttp3 in a new package shadow.okhttp3. Why you do not depend on the 3d party libraries like

api  'com.squareup.okhttp3:okhttp:3.12.0'

instead of creating a copy? now, I will have two instances of okhttp class the one shadowed and the non shadowed. and other clashes will appear, for example:

More than one file was found with OS independent path 'META-INF/proguard/okhttp3.pro'
bartekn commented 5 years ago

Please check https://github.com/stellar/java-stellar-sdk/issues/122, #123 and #132 for reasoning behind shadowing dependencies. When it comes to the new issue you are encountering, does this Stack Overflow answer help?

dabitdev commented 5 years ago

I understand that you have issues supporting Spring Boot. How about creating a specific sdk for android?

@bartekn excluding the META-INF is a work around. Since I will end up having two different versions of shadow.okhttp your shadowed and the okhttp used in my application layer, you should rename the proguard rules files.

jillesvangurp commented 5 years ago

@bartekn now that we have gotten rid of jersey, we can try rolling back the shadowing. The spring boot problems were related to that. The main remaining issue would be the android specific guava dependency. I would argue simply removing that and working around the few places where it was actually needed. I think the main reason was base64 encoding/decoding. There are other libraries we can use for that.

dabitdev commented 5 years ago

@jillesvangurp are you still going to keep the fat JAR/AAR approach? In Android world using gradle and AAR library, does not make a lot of sense to copy the 3d libraries inside.

jillesvangurp commented 5 years ago

I'm a backend guy, I know nothing of AARs. But please if you have time, doing a PR to get rid of shading and the android guava dependency would unblock this. I'm bogged down in other stuff currently so no time to do this myself but should be fairly easy to do.

bartekn commented 5 years ago

How about creating a specific sdk for android?

Java SDK is updated quite often so I think having two SDKs that are almost the same (except small details to make it work in Android) will make it super hard to keep everything in sync. Obviously if there's no other way we'll have to do it but I really want to avoid it. And it seems that except some small problems you can still use it in Android, right?

Since I will end up having two different versions of shadow.okhttp your shadowed and the okhttp used in my application layer

I think this is actually good. Let's say you upgrade your okhttp version in your application. If there are any breaking changes Java SDK will stop working because of the changed API.

dabitdev commented 5 years ago

Most SDKs expose all the dependencies, I do not see the benefit of shadowing libraries here. Imagine that I have 3 SDKs like Stellar, I will end up having 3 different version of the network stack, do you think this is ideal? Having n Okhttp versions make things difficult to manage. I will have to implement everything related to network n times, for example: network interceptors, ssl pinning, mocking, security patches, ...

On the other hand, @jillesvangurp said that shadowing is not needed anymore? The only benefit of keeping like that is avoiding version conflicts between SDKS, applications using different versions of the 3rd party libraries is a normal but not enjoyable duty. Every SDK and Application should be encourage to upgrade the 3rd party libraries.

Meshkati commented 5 years ago

Please check #122, #123 and #132 for reasoning behind shadowing dependencies. When it comes to the new issue you are encountering, does this Stack Overflow answer help?

The StackOverflow answer is not working for me and I still get More than one file was found with OS independent path 'META-INF/proguard/okhttp3.pro', is there any other solution to fix the conflict with okhttp3? Like making JARs or something?

dabitdev commented 4 years ago

Hi @Meshkati , you can check blockEQ project I solved that issue there.

overcat commented 1 year ago

Hi @sreuland, I think now we can pick up this issue again. Personally, I also prefer not to use shadow. What do you think?

sreuland commented 1 year ago

yes, removing shadowing would be compelling to reduce complexity of distribution and give some control of overlapping dependencies back to the user through transitive dependency management, sounds like worth trying it out in a draft pr to see how it shapes up, another issue to carve out may be to consider publishing the lib to maven central repo rather than jitpack, so, users don't have to add that repo to their build, as maven central is included by default.

there have been multiple issues reported on class loader with okhttp, shadowing just makes it more complicated, https://github.com/stellar/java-stellar-sdk/pull/457 was another somewhat recent pr on related fix.

orthogonal to shadowing, but to mention in context here, there is also modernization of dependencies on https://github.com/stellar/java-stellar-sdk/issues/496, which is suggesting to remove okhttp in any case in favor of much lower footprint of native http client instead which could alleviate some of this from a different angle.

overcat commented 1 year ago

We have removed shadow in 0.41.0.