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

Update groovy 3 #590

Closed
wants to merge 2 commits into from
Closed

Update groovy 3 #590

wants to merge 2 commits into from

Conversation

FSchumacher
Copy link
Contributor

@FSchumacher FSchumacher commented May 7, 2020

Description

Update Groovy from 2.4 to 3.0 and use the split up jars instead of the groovy-all one.

Note The test framework spock – that we use in our test base – currently supports Groovy 3.0 only in the 2.0 branch, which is not officially released, yet.

I am not sure, whether this PR should be merged before the next release.

Motivation and Context

Groovy 3.0 is the current development line of Groovy and will support newer Java versions better.

How Has This Been Tested?

./gradlew test ran successfully on my machine

Screenshots (if appropriate):

Types of changes

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.

apiv("org.codehaus.groovy:groovy", "groovy")
apiv("org.codehaus.groovy:groovy-datetime", "groovy")
apiv("org.codehaus.groovy:groovy-jmx", "groovy")
apiv("org.codehaus.groovy:groovy-json", "groovy")
apiv("org.codehaus.groovy:groovy-jsr223", "groovy")
apiv("org.codehaus.groovy:groovy-sql", "groovy")
apiv("org.codehaus.groovy:groovy-templates", "groovy")
Copy link
Collaborator

@vlsi vlsi May 7, 2020

Choose a reason for hiding this comment

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

Can you please use org.codehaus.groovy:groovy-bom instead?

Suggested change
apiv("org.codehaus.groovy:groovy", "groovy")
apiv("org.codehaus.groovy:groovy-datetime", "groovy")
apiv("org.codehaus.groovy:groovy-jmx", "groovy")
apiv("org.codehaus.groovy:groovy-json", "groovy")
apiv("org.codehaus.groovy:groovy-jsr223", "groovy")
apiv("org.codehaus.groovy:groovy-sql", "groovy")
apiv("org.codehaus.groovy:groovy-templates", "groovy")
api(platform("org.codehaus.groovy:groovy-bom:${"groovy".v}"))

Copy link
Contributor Author

@FSchumacher FSchumacher May 7, 2020

Choose a reason for hiding this comment

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

That looks really nice, but when I try it out, it gives me the following error:

Build jmeter FAILURE reason:                                
    org.gradle.internal.exceptions.LocationAwareException: Build file '/home/felix/Developer/fschumacher-jmeter/src/bom/build.gradle.kts' line: 90
    Cannot convert the provided notation to an object of type DependencyConstraint: DefaultExternalModuleDependency{group='org.codehaus.groovy', name='groovy-bom', version='3.0.3', configuration='default'}.
    The following types/formats are supported:
      - Instances of DependencyConstraint.
      - String or CharSequence values, for example 'org.gradle:gradle-core:1.0'.
      - Maps, for example [group: 'org.gradle', name: 'gradle-core', version: '1.0'].
      - Projects, for example project(':some:project:path').
      - Instances of ProjectDependency.
    
    Comprehensive documentation on dependency notations is available in DSL reference for DependencyHandler type.
        Caused by: org.gradle.internal.typeconversion.UnsupportedNotationException: Cannot convert the provided notation to an object of type DependencyConstraint: DefaultExternalModuleDependency{group='org.codehaus.groovy', name='groovy-bom', version='3.0.3', configuration='default'}.
        The following types/formats are supported:
          - Instances of DependencyConstraint.
          - String or CharSequence values, for example 'org.gradle:gradle-core:1.0'.
          - Maps, for example [group: 'org.gradle', name: 'gradle-core', version: '1.0'].
          - Projects, for example project(':some:project:path').
          - Instances of ProjectDependency.

Copy link
Collaborator

@vlsi vlsi May 7, 2020

Choose a reason for hiding this comment

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

Oh,

https://docs.gradle.org/current/userguide/java_platform_plugin.html#sec:java_platform_bom_import

So there should be the following before dependencies

javaPlatform {
    allowDependencies()
}

and api(platform(...)) should come in the dependencies section rather than dependencies { constraints { ... }}.

Can you please try that? (move api(platform before (constraints) {...)

Copy link
Contributor Author

@FSchumacher FSchumacher May 7, 2020

Choose a reason for hiding this comment

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

That works. Thanks for the hint.

@vlsi
Copy link
Collaborator

@vlsi vlsi commented May 7, 2020

Just in case: I think 2.0-M2-groovy-3.0 is OK Spock version to use.
I don't think we bend Spock to the extreme, so I don't think we would suffer from further Spock upgrades.

Copy link
Contributor

@pmouawad pmouawad left a comment

I would just add in release notes in section breaking change a note about this.

Thanks for PR

@asfgit asfgit closed this in da402ca May 9, 2020
@FSchumacher FSchumacher deleted the update-groovy-3 branch May 9, 2020
kkalinin pushed a commit to kkalinin/jmeter that referenced this issue Mar 11, 2021
As a side-effect, we have to update Spock to 2.0-M2
Closes apache#590
kkalinin pushed a commit to kkalinin/jmeter that referenced this issue Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants