hyperledger-web3j / web3j-solidity-gradle-plugin

Gradle plugin providing tasks to compile Solidity contracts.
Apache License 2.0
21 stars 25 forks source link

Make plugin work with Gradle 7.0; #39

Closed eliquinox closed 2 years ago

eliquinox commented 3 years ago

What does this PR do?

This PR bumps Gradle version to 7.0 and makes according adjustments, making the plugin compatible with Gradle 7.0

Where should the reviewer start?

Some adjustment were made to tests because implicit dependency between task compileSolidity and jar meant that compile Solidity produced SUCCESS instead of UP_TO_DATE when re-executed (https://docs.gradle.org/7.0/userguide/validation_problems.html#implicit_dependency). I have fixed this my modifying test build files and keeping the same assertions. You may find a more elegant solution to this.

Why is it needed?

To make plugin compatible with latest major version of Gradle.

xaviarias commented 3 years ago

Hi and thanks for the PR.

Is the incremental build issue happening also on the 0.3.2 version or is something introduced by Gradle 7?

Normally there should be no dependency between JAR and compile Solidity tasks.

eliquinox commented 3 years ago

Hi and thanks for the PR.

Is the incremental build issue happening also on the 0.3.2 version or is something introduced by Gradle 7?

Normally there should be no dependency between JAR and compile Solidity tasks.

The incremental build issue is introduced by Gradle 7.0 and you can check the link in the description to find out more. It looks like Gradle 7.0 takes JAR and source tasks as implicitly dependent.

eliquinox commented 3 years ago

Hi. Can this PR be given a verdict?

kare commented 3 years ago

I'm using this PR with Gradle 7.0.2 and it works great!

xaviarias commented 3 years ago

Hi, I have two objections coming out of my head.

One is backwards compatibility. Does this PR work with a Gradle 5.x or 6.x project?

Second is those Gradle file changes in the unit tests, eg:

tasks {
    jar {
          // deal with implicit dependendency issue so that
          // compileSolidity can be "UP-TO-DATE" when re-ran
          mustRunAfter "compileSolidity"
     }
}

Would the end user need to add these blocks to their Gradle files?

If yes, that is a blocker since all Web3j tools are built with sensitive defaults to hide the intricacies to the user.

In that case, the plugin would need to take care of these tasks dependencies.

kare commented 3 years ago

@eliquinox What do you think?

eliquinox commented 3 years ago

@xaviarias Thanks for review. Addressing your concerns:

  1. Backward compatibility: this change is compatible with version 6.0 but not 5.0. On the latter it fails with:

    An exception occurred applying plugin request [id: 'org.web3j.solidity']
    > Failed to apply plugin [class 'com.github.gradle.node.NodePlugin']
    > Could not create an instance of type com.github.gradle.node.NodeExtension_Decorated.
      > 'org.gradle.api.file.DirectoryProperty org.gradle.api.file.DirectoryProperty.convention(org.gradle.api.file.Directory)'

    Note that it fails in the same way on the current trunk if Gradle 5.0 is used.

  2. As I understand, the users of plugin should not see any difference in correctness of their build scripts. Unit tests tell you the following: if before a user would have expected his task to have "UP-TO-DATE" status, now he will only be able to achieve this by supplying the change to their jar task config as per updated unit tests. I cannot see any difference except different task status being shown in the gradle output log. If you have a project in mind that uses this plugin and will suffer as a result of this change, do test it out and let me know of the results.

eliquinox commented 3 years ago

@xaviarias let me know if you have any further reservations about this. Otherwise, let's get this merged.

kare commented 3 years ago

@xaviarias Any comments?

cc: @eliquinox

AtomaszewskiFP commented 3 years ago

@xaviarias any update on this?

xaviarias commented 3 years ago

Hi, yes this has been included in the roadmap. Will be working on this week and keep this thread updated.

marko-lazic commented 3 years ago

Until this gets merged, how to add your project to my gradle build?

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.