-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Port the latest web3j release (on master) to Java 1.6 in preparation for an Android release #769
Comments
Out of interest why does the Android code need to be using Java 1.6? From the Android documentation.
|
We have a significant number of developers that are targeting older versions of Android in their applications, so we'd like to keep them in the web3j builds and offerings. |
@snazha-blkio from what I've read it seems that AS 3.0+ will support Java 1.7 for all Android versions. I know in the latest stable build for AS there is good support for Java 1.8 as well but it would be worthwhile having this tested to see if it's viable. It just seems like you're giving yourself more work than necessary when you could just support Java 1.7 and let all the developers upgrade to the latest stable version of AS. I would like to think that reducing the effort required to convert to Java 1.6 would result in easier and faster releases for Web3j for Android. |
Valid point @MarkOSullivan94 re: AS 3.0. I will take a look at AS 3.0 as it has good support for some Java 8 classes that are used profusely (e.g. CompletableFuture). I'll post my findings thereafter. Thanks for the feedback! |
As @MarkOSullivan94 already mentioned, "desugar" procedure, built-in Android's build pipeline, handles some Java 8 language features, such as lambda expressions, method references, default and static interface methods and repeating annotations. It produces Java 6 compatible bytecode from Java 8 code. However, some Java 8 APIs are not supported on older phones, such as My suggestion: use Java 8 language features but replace incompatible APIs. A good library that can help with that is Guava. Note also that Java 8 compatibility is not the only one problem on Android. Other issues are:
|
Issue Status: 1. Open 2. Started 3. Submitted 4. Done This issue now has a funding of 300.0 DAI (300.0 USD @ $1.0/DAI) attached to it as part of the Ethereum Foundation fund.
|
I've applied, I wasn't sure if it was allowed to use a library to solve the issue, please let me know @snazha-blkio |
Our concerns around using a library are around licensing and ensuring full android compatibility. Thus, we'd rather no library be used. |
@snazha-blkio Then I don't think I'm the man for the job - Sorry 😄 |
@snazha-blkio, I was interested in this task and even started experimenting with the code. It seems to me that the simplest way to make Web3j Android compatible is, as mentioned before, to port only missing APIs (method, classes and packages) leaving lambdas and other Java 8 features intact. |
@serso That is exactly what I experimented with as well.. |
Code in the task branch is now binary compatible with Android JDK. Streamsupport library is used only in "core" module. |
hey @serso, Frank from Gitcoin here, could you please hit "start work" here: https://gitcoin.co/issue/web3j/web3j/769/1777 so @ceresstation can approve you for work (since you've started already?) :) |
@frankchen07 done :) |
Hi, sorry just catching up on this. web3j needs to use Apache 2 friendly libraries. Pulling in GPLv2 is problematic for this reason. |
@conor10 please read #769 (comment) |
@serso I think you're correct with regards to licensing. Can we proceed on that assumption and get everything else ready? I think we just need to double check with the lawyers before we hit merge. |
@antonydenyer yes, we can proceed. Current status: https://github.com/serso/web3j/commits/my-android The only remaining thing left to do in the code is to abstract crypto provider API. The bundled version of Bouncy Castle in Android is crippled and doesn't contain ciphers used by web3j. It has to be substituted by Spongy Castle in Android. I patched it myself here but we need a more maintainable solution than that. My plan is to create a submodule (or a separate project) with a sample Android app to verify that the library works as expected. I would like to add a set of Proguard rules as well. Proguard is often used in Android to obfuscate and minimize the code. I know that web3j has several issues with it. I also can take a look into removing |
@snazha-blkio @conor10 note also that in my WIP branch I didn't convert Java 8 language features supported on Android, such as lambdas. Please confirm that this is okay (the task description says "Port everything to Java 1.6"). |
I think if developers want to do this when deploying their applications that's their business, not ours. |
On the other hand it might be difficult to write Proguard rules if you are not familiar with web3j. |
@serso Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days |
Status update: I've created a test Android app with the binaries built from the tip of the task branch. The app seems to work fine on Pixel and Moto G. It seems that there are no issues neither with XML APIs nor with Bouncy Castle (BC) mentioned above. BC issue seems to be resolved due to this. I haven't tried signing yet but the changes seem to be promising. Implementing signing in the test app is the next step for me. @snazha-blkio @conor10 can you please follow up on this and this? |
You have answered your own question stick with bc
👍
@conor @snazha please confirm that the license of streamsupport is compatible. It would be nice to have this on the android branch to make porting less of a burden in the future
I have no idea on this... |
My goal was to force devs to write Android compatible code from the beginning. If this PR lands on master only features supported on Android will be available for development. Of course, It's not a problem to port these changes into the release branch and prepare a "4.0.0-android" package for the corresponding "4.0.0" release. I think it's better if we avoid releasing Android specific packages in the future by making the main package Android compatible. And this PR ensures that. |
I'm done with testing. Everything seems work fine on Android API 21+. Things that have been tested:
Test app: https://github.com/serso/web3a |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done Work for 300.0 DAI (300.0 USD @ $1.0/DAI) has been submitted by: @ceresstation please take a look at the submitted work:
|
When the review is done I'll rebase the task branch on |
Hey guys, It would be nice if you could provide some feedback on the PR. You don't need to review the whole code, just check out the approach and confirm that it's okay. |
Fixed in #809 |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done The funding of 300.0 DAI (300.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @serso.
|
@snazha-blkio when do you plan to release the Android version? I tried some snapshot artifacts (abi, compat, crypto, rlp and utils) and they seem to work fine on Android. |
4.1.0-android has just been released. |
Hi @serso Would you have any issues if we took a cut of your web3a (https://github.com/serso/web3a) code and published it in our web3j repo as a sample android app? The idea is to build upon it to have a good app to test functionality with. Please let us know if have any issues with it. Thanks. |
@snazha-blkio No, I don't mind that. |
Background
Provide a port of the web3j 4.0 code to Java 1.6 in order to support an Android release.
The last web3j release for android was version 3.3.1 (refer to the "master-android" branch), a new release for Android needs to be cut.
Objective
To port the Java 1.7 and 1.8 portions of the web3j codebase to 1.6 equivalent constructs
Requirements
Technical details
Next steps
There are two options.
Should you choose the second path ...
The text was updated successfully, but these errors were encountered: