Skip to content
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

Gradle 7.1.1 compat #32

Closed
wants to merge 7 commits into from

Conversation

jhyry-gcpud
Copy link

Fix for compatibility with Gradle 7.1.1.

Added eclipse project settings for the buildship plugin and groovy.
Ignore bin and classpath+project files since Gradle generates these.
Added gradle.properties to enable caching and specify JVM args.

Fix issues with Gradle 7.1.1 in the Groovy plugin code, including:
 - Add getters for newly @Input/@internal properties.
 - Add @input and @internal appropriately to properties to comply with 7.1.1.
Remove jcenter bintray as repo since it got shut down.
Fix gradle migration issues:
 - Work around gradle duplicate-no-duplicate issue by
    including duplicates.
 - testCompile to testCompileOnly, since testCompile
    got completely removed in 7.0.0.
Add eclipse plugin with SpotBugs nature+builder.
@eerohele
Copy link
Owner

There's a bunch of changes here that don't seem related to Gradle 7.1.1 compatibility. The Eclipse stuff, for instance.

Can you explain how the plugin currently isn't compatible with Gradle 7.1.1 and how the changes here fix it?

Removed eclipse sections that are not required for Gradle 7.1.1.
@jhyry-gcpud
Copy link
Author

@eerohele Good point. I can remove that stuff. I have the following list of changes required to make the build compatible with Gradle 7+:

  • Wrapper upgrade:
    diff --git a/build.gradle b/build.gradle
    index 4fe96db..03449b4 100644
    --- a/build.gradle
    +++ b/build.gradle
    @@ -33,7 +31,7 @@ test {
     }
    
     wrapper {
    -    gradleVersion = '6.0'
    +    gradleVersion = '7.1.1'
     }
    
     codenarc {
    
    diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties
    index 35cfa32..05679dc 100644
    --- a/gradle/wrapper/gradle-wrapper.properties
    +++ b/gradle/wrapper/gradle-wrapper.properties
    @@ -1,6 +1,5 @@
    -#Tue Nov 12 23:08:13 EET 2019
     distributionBase=GRADLE_USER_HOME
     distributionPath=wrapper/dists
    +distributionUrl=https\://services.gradle.org/distributions/gradle-7.1.1-bin.zip
     zipStoreBase=GRADLE_USER_HOME
     zipStorePath=wrapper/dists
    -distributionUrl=https\://services.gradle.org/distributions/gradle-6.0-all.zip
    
  • Gradle 7+ uses Groovy 3, which is incompatible with the version of Spock this project is using, thus the Spock upgrade to a compatible version:
    image
  • A workaround is required to get the prepareResources task to complete successfully.
    The reason for the following workaround is described here and here:
    // Temporary workaround for phantom duplicates in Gradle 7+.
    rootProject.tasks.named("processResources") {
         duplicatesStrategy = 'include'
    }
    

Let me know if this is sufficient information or not.
Thanks so much for taking a look at this!

Jon

Need a repo to get deps. Whoops.
@jhyry-gcpud
Copy link
Author

The result of running the build on 7.1.1 is the following, with --no-build-cache and --rerun-tasks to ensure everything works:

PS saxon-gradle> ./gradlew clean build --no-build-cache --rerun-tasks

> Task :groovydoc
Trying to override old definition of task fileScanner
The Cobertura XML file [null] is not accessible; skipping this rule
The Cobertura XML file [null] is not accessible; skipping this rule

BUILD SUCCESSFUL in 33s
15 actionable tasks: 15 executed

Here's the build result:
saxon-gradle_7.1.1_build.zip

@jhyry-gcpud
Copy link
Author

In regard to src/main/groovy/com/github/eerohele/SaxonXsltTask.groovy, the plugin would not build in 7.1.1 without the added annotations and getters.

These are not required for Gradle 7+.
@jhyry-gcpud
Copy link
Author

@eerohele Is this PR ok now?

@eerohele
Copy link
Owner

@jhyry-gcpud Sorry for the delay. Life keeps getting in the way.

I looked into it a bit, and I think I fixed most of the Gradle 7 compatibility issues. I've published v0.9.0-beta4 into the Gradle plugin portal if you want to give it a try.

After these changes, I don't get any deprecation warnings regarding the @Input or @Internal stuff. If there's a problem that requires them to be added that you'd like to get fixed, please make a new PR that only adds those attributes, with an explanation of the problem it fixes.

@eerohele eerohele closed this Aug 28, 2021
@jhyry-gcpud
Copy link
Author

I very much understand life getting in the way.
Thanks again so much for looking at this and working out the underlying issues, I can most definitely give this a try sometime in the next week or 2.
Cheers!

@jhyry-gcpud
Copy link
Author

@eerohele Works like a charm! Thanks again!

@eerohele
Copy link
Owner

eerohele commented Sep 8, 2021

Cool, thanks for reporting back!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants