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

fix XmlSlurper deprecation #656

Merged
merged 1 commit into from
Sep 15, 2022
Merged

fix XmlSlurper deprecation #656

merged 1 commit into from
Sep 15, 2022

Conversation

jaredsburrows
Copy link
Collaborator

@jaredsburrows jaredsburrows commented May 10, 2022

  • groovy.util.XmlSlurper -> groovy.xml.XmlSlurper
  • groovy.util.slurpersupport.GPathResult -> groovy.xml.slurpersupport.GPathResult
  • groovy.util.slurpersupport.NodeChildren -> groovy.xml.slurpersupport.NodeChildren
  • Use non deprecated groovy.xml.XmlParser

@ben-manes

@jaredsburrows
Copy link
Collaborator Author

jaredsburrows commented May 11, 2022

@ben-manes

> Configure project :
e: /Users/<>/repo/gradle-versions-plugin/api/build/libs/api.jar!/com/github/benmanes/gradle/versions/updates/resolutionstrategy/ResolutionStrategyWithCurrent.class: Class 'com/github/benmanes/gradle/versions/updates/resolutionstrategy/ResolutionStrategyWithCurrent' was compiled with an incompatible version of Kotlin. The binary version of its bytecode is unknown, expected version is 1.0.3
e: /Users/<>/repo/gradle-versions-plugin/api/build/libs/api.jar!/com/github/benmanes/gradle/versions/updates/resolutionstrategy/ComponentSelectionRulesWithCurrent.class: Class 'com/github/benmanes/gradle/versions/updates/resolutionstrategy/ComponentSelectionRulesWithCurrent' was compiled with an incompatible version of Kotlin. The binary version of its bytecode is unknown, expected version is 1.0.3
e: /Users/<>/repo/gradle-versions-plugin/api/build/libs/api.jar!/com/github/benmanes/gradle/versions/updates/resolutionstrategy/ComponentSelectionWithCurrent.class: Class 'com/github/benmanes/gradle/versions/updates/resolutionstrategy/ComponentSelectionWithCurrent' was compiled with an incompatible version of Kotlin. The binary version of its bytecode is unknown, expected version is 1.0.3

@DPUkyle
Copy link

DPUkyle commented Sep 14, 2022

@ben-manes do you mind rerunning the CI actions and merging?

@jaredsburrows
Copy link
Collaborator Author

@DPUkyle Try syncing with the latest code and push back up.

@DPUkyle
Copy link

DPUkyle commented Sep 14, 2022

@DPUkyle Try syncing with the latest code and push back up.

I think you'd need to add me as a collaborator on your fork: https://github.com/jaredsburrows/gradle-versions-plugin/settings/access

@jaredsburrows
Copy link
Collaborator Author

@DPUkyle I did not see the "re-run failed" jobs, so I just synced it now.

@DPUkyle
Copy link

DPUkyle commented Sep 14, 2022

I understand the failure cause. Tests in com.github.benmanes.gradle.versions.KotlinDslUsageSpec are parameterized to run with Gradle 5.6, despite the plugin being compiled against Gradle 7.4.2's API. Gradle 5.6 bundles Groovy 2.5.4, whereas Gradle 7.x bundles Groovy 3.x, which introduces the replacements for split packages. In other words, the failure is correct as groovy.xml.XmlSlurper does not exist when run with Gradle 5.x and 6.x.

Off the top of my head, I see two options:

  1. Require Gradle 7 and higher to apply the plugin
  2. Within the Resolver class, use GroovySystem#version to get the Groovy version, then use reflection to invoke the appropriate API: groovy.util.XmlSlurper or groovy.xml.XmlSlurper

WDYT?

PS The table in gradle/gradle#17375 has a nice illustration of bundled Groovy and Kotlin versions.

@ben-manes
Copy link
Owner

I am fine if we bump up the minimum version of Gradle. That seems preferable to reflection workarounds.

@DPUkyle
Copy link

DPUkyle commented Sep 14, 2022

@jaredsburrows please up the minimum version here.

This patch will run KotlinDslUsageSpec remove 5.6 and run the tests with the same version of Groovy as the Gradle wrapper uses:

Index: gradle-versions-plugin/src/test/groovy/com/github/benmanes/gradle/versions/KotlinDslUsageSpec.groovy
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/gradle-versions-plugin/src/test/groovy/com/github/benmanes/gradle/versions/KotlinDslUsageSpec.groovy b/gradle-versions-plugin/src/test/groovy/com/github/benmanes/gradle/versions/KotlinDslUsageSpec.groovy
--- a/gradle-versions-plugin/src/test/groovy/com/github/benmanes/gradle/versions/KotlinDslUsageSpec.groovy	(revision b2277b57d97029208e4de3ce481da8ffe37e92d6)
+++ b/gradle-versions-plugin/src/test/groovy/com/github/benmanes/gradle/versions/KotlinDslUsageSpec.groovy	(date 1663121891204)
@@ -37,8 +37,7 @@
         """.stripIndent()
   }
 
-  @Unroll
-  def "user friendly kotlin-dsl with #gradleVersion"() {
+  def "user friendly kotlin-dsl"() {
     given:
     buildFile << '''
       tasks.named<DependencyUpdatesTask>("dependencyUpdates") {
@@ -60,7 +59,6 @@
 
     when:
     def result = GradleRunner.create()
-      .withGradleVersion(gradleVersion)
       .withPluginClasspath()
       .withProjectDir(testProjectDir.root)
       .withArguments('dependencyUpdates')
@@ -69,9 +67,6 @@
     then:
     result.output.contains('''com.google.inject:guice [2.0 -> 3.0]''')
     result.task(':dependencyUpdates').outcome == SUCCESS
-
-    where:
-    gradleVersion << ['5.6']
   }
 
   @Unroll
@@ -97,7 +92,6 @@
 
     when:
     def result = GradleRunner.create()
-      .withGradleVersion('5.6')
       .withPluginClasspath()
       .withProjectDir(testProjectDir.root)
       .withArguments('dependencyUpdates')

@DPUkyle
Copy link

DPUkyle commented Sep 14, 2022

This is odd - your changes look good to me, but the checks fail running tests that have been removed from the class: https://scans.gradle.com/s/y2ohuqhdfmaf4/tests/overview?outcome=failed

Updated: I think your fork is out-of-sync. I see the failing test still exists on master.

@jaredsburrows
Copy link
Collaborator Author

I still see

    e: /home/runner/work/gradle-versions-plugin/gradle-versions-plugin/gradle-versions-plugin/build/classes/kotlin/main/com/github/benmanes/gradle/versions/updates/DependencyUpdatesTask.class: Class 'com/github/benmanes/gradle/versions/updates/DependencyUpdatesTask' was compiled with an incompatible version of Kotlin. The binary version of its bytecode is unknown, expected version is 1.0.3

@DPUkyle
Copy link

DPUkyle commented Sep 15, 2022

I still see

    e: /home/runner/work/gradle-versions-plugin/gradle-versions-plugin/gradle-versions-plugin/build/classes/kotlin/main/com/github/benmanes/gradle/versions/updates/DependencyUpdatesTask.class: Class 'com/github/benmanes/gradle/versions/updates/DependencyUpdatesTask' was compiled with an incompatible version of Kotlin. The binary version of its bytecode is unknown, expected version is 1.0.3

Yeah, that's strange and also I think the GitHub Action is checking out the wrong git reference, hence the test still failing.

@bigdaz
Copy link
Contributor

bigdaz commented Sep 15, 2022

@DPUkyle asked me to take a look at this failure based on my GitHub actions experience.

If this PR can be build locally, but is failing consistently on CI, then I suspect that there's some state in Gradle User Home that isn't being correctly invalidated. I don't really understand what state would cause this, but it could explain why this is a persistent failure on CI that isn't seen locally. (The gradle-build-action will restore Gradle User Home on each execution based on a previous run).

There are a couple things that could fix this:

  1. Temporarily disable caching in build.yml: https://github.com/gradle/gradle-build-action#caching
  2. Bump the PR to run with Gradle 7.5.1 (much of the state in Gradle User Home is tied to Gradle version). This could be as simple as rebasing this PR on top of gradle 7.5.1 #694.

@ben-manes
Copy link
Owner

okay, merged #694. Please rebase and try again.

@bigdaz
Copy link
Contributor

bigdaz commented Sep 15, 2022

Looks like an updated Gradle version didn't help. Next thing to try would be running the workflow with cache-disabled: true. That will cause the CI build to run with a fresh Gradle User Home.

@vjgarciag96
Copy link
Collaborator

vjgarciag96 commented Sep 15, 2022

I think we are missing the removal of this line (https://github.com/ben-manes/gradle-versions-plugin/blob/master/gradle-versions-plugin/src/test/groovy/com/github/benmanes/gradle/versions/KotlinDslUsageSpec.groovy#L100) if the problem in the other test was the Gradle version.

@bigdaz @ben-manes

Copy link

@DPUkyle DPUkyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yank this line please:

@@ -97,7 +92,6 @@
 
     when:
     def result = GradleRunner.create()
-      .withGradleVersion('5.6')
       .withPluginClasspath()
       .withProjectDir(testProjectDir.root)
       .withArguments('dependencyUpdates')

@jaredsburrows
Copy link
Collaborator Author

@DPUkyle Will update this PR and resolve the rest here - #695.

@jaredsburrows
Copy link
Collaborator Author

Thank you @DPUkyle @bigdaz!

@jaredsburrows jaredsburrows merged commit 0e5660e into ben-manes:master Sep 15, 2022
@jaredsburrows jaredsburrows deleted the pr/jaredsburrows/fix-XmlSlurper-deprecation branch September 15, 2022 20:39
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.

5 participants