grails-core icon indicating copy to clipboard operation
grails-core copied to clipboard

7.0.x dependency cleanup

Open codeconsole opened this issue 1 year ago • 31 comments
trafficstars

I think this will be a lot easier to maintain. All version numbers have been pulled out and the entire grails-core repo now uses its own bom for dependency version resolution.

I provided a more concise, clean approach to listing dependency versions. The previous way was a bit more tedious and time consuming having to add a map entry for every dependency.

Check out those deletion stats 😉

Still work needs to be done to analyze dependency configurations, but this can be done in a later ticket.

Spring Bom Groovy Bom

codeconsole avatar Oct 18 '24 01:10 codeconsole

The build is failing after the last cleanup commit 80534e7. Could not find jline:jline: and Could not find com.github.javaparser:javaparser-core:

matrei avatar Oct 18 '24 06:10 matrei

The build is failing after the last cleanup commit 80534e7

I guess it doesn't like to resolve bom versions for documentation dependencies

        documentation("org.fusesource.jansi:jansi:$jansiVersion")
        documentation("jline:jline:$jlineVersion")
        documentation ("com.github.javaparser:javaparser-core:$javaParserCoreVersion")

I added reverted the versions back.

codeconsole avatar Oct 18 '24 07:10 codeconsole

I guess it doesn't like to resolve bom versions for documentation dependencies

Wouldn't documentation platform(project(':grails-bom')) add it?

matrei avatar Oct 18 '24 08:10 matrei

Wouldn't documentation platform(project(':grails-bom')) add it? @matrei apparently so... done.

codeconsole avatar Oct 18 '24 09:10 codeconsole

Wouldn't documentation platform(project(':grails-bom')) add it? apparently so... done

Did you push that change?

matrei avatar Oct 18 '24 09:10 matrei

Wouldn't documentation platform(project(':grails-bom')) add it? apparently so... done

Did you push that change?

@matrei yeah, it worked

codeconsole avatar Oct 18 '24 10:10 codeconsole

Could we put the version properties at the top of the pom, like in https://repo1.maven.org/maven2/org/springframework/boot/spring-boot-dependencies/3.3.4/spring-boot-dependencies-3.3.4.pom? (That's where the important information is)

matrei avatar Oct 18 '24 11:10 matrei

Could we add spring-boot.version so it's possible to override that too? Does that make sense?

(dependencyVersions.collect { [ version:it.key.version, name: "${it.key.name}.version" ] } +
    pluginsAndProfiles + [name: 'spring-boot.version', version: springBootVersion]
).sort { it.name }
    .groupBy { it.name }.collect { it.value.first() } // remove duplicates
    .each { propsNode.appendNode(it.name, it.version) }
deps.appendNode('dependency').with {
    appendNode('groupId', 'org.springframework.boot')
    appendNode('artifactId', 'spring-boot-dependencies')
    appendNode('version', '${spring-boot.version}')
    appendNode('type', 'pom')
    appendNode('scope', 'import')
}

matrei avatar Oct 18 '24 11:10 matrei

Shouldn't all the modules in grails-core.ROOT also be in the bom? Otherwise you can't do this for example: implementation 'org.grails:grails-web-url-mappings'

matrei avatar Oct 18 '24 12:10 matrei

org.junit.jupiter:junit-jupiter:api,engine:$junitVersion
org.junit.platform:junit-platform-runner::$junitPlatformRunnerVersion

should be removed from the bom, they are already in the spring-boot bom and the versions mismatch so there is an error when running tests:

org.junit.platform.commons.JUnitException: TestEngine with ID 'junit-jupiter' failed to discover tests

matrei avatar Oct 18 '24 12:10 matrei

org.junit.jupiter:junit-jupiter:api,engine:$junitVersion
org.junit.platform:junit-platform-runner::$junitPlatformRunnerVersion

should be removed from the bom, they are already in the spring-boot bom and the versions mismatch so there is an error when running tests:

org.junit.platform.commons.JUnitException: TestEngine with ID 'junit-jupiter' failed to discover tests

How/wheere are you getting an error running tests?

codeconsole avatar Oct 18 '24 18:10 codeconsole

How/wheere are you getting an error running testts?

I'm publishing this branch to mavenLocal and using it to build grails-mail for testing. With the junit versions in the grails-bom I get below error stack, without the tests run without issues. (Related: https://stackoverflow.com/questions/70452633/org-junit-platform-commons-junitexception-testengine-with-id-junit-jupiter-fa)

org.gradle.api.internal.tasks.testing.TestSuiteExecutionException: Could not complete execution for Gradle Test Executor 2.
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:65)
	at [email protected]/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at [email protected]/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at [email protected]/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at [email protected]/java.lang.reflect.Method.invoke(Method.java:569)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:92)
	at jdk.proxy1/jdk.proxy1.$Proxy4.stop(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:200)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:132)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:103)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:63)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:121)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71)
	at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
Caused by: org.junit.platform.commons.JUnitException: TestEngine with ID 'junit-jupiter' failed to discover tests
	at app//org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:160)
	at app//org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverSafely(EngineDiscoveryOrchestrator.java:132)
	at app//org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:107)
	at app//org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:78)
	at app//org.junit.platform.launcher.core.DefaultLauncher.discover(DefaultLauncher.java:99)
	at app//org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:85)
	at app//org.junit.platform.launcher.core.DelegatingLauncher.execute(DelegatingLauncher.java:47)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:124)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:99)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:94)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:63)
	... 18 more
Caused by: org.junit.platform.commons.JUnitException: ClassSelector [className = 'grails.plugins.mail.MailMessageBuilderSpec', classLoader = jdk.internal.loader.ClassLoaders$AppClassLoader@4e0e2f2a] resolution failed
	at app//org.junit.platform.launcher.listeners.discovery.AbortOnFailureLauncherDiscoveryListener.selectorProcessed(AbortOnFailureLauncherDiscoveryListener.java:39)
	at app//org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.resolveCompletely(EngineDiscoveryRequestResolution.java:103)
	at app//org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.run(EngineDiscoveryRequestResolution.java:83)
	at app//org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolver.resolve(EngineDiscoveryRequestResolver.java:113)
	at app//org.junit.jupiter.engine.discovery.DiscoverySelectorResolver.resolveSelectors(DiscoverySelectorResolver.java:48)
	at app//org.junit.jupiter.engine.JupiterTestEngine.discover(JupiterTestEngine.java:69)
	at app//org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:152)
	... 28 more
Caused by: java.lang.NoSuchMethodError: 'boolean org.junit.platform.commons.util.ReflectionUtils.returnsPrimitiveVoid(java.lang.reflect.Method)'
	at org.junit.jupiter.engine.discovery.predicates.IsTestableMethod.test(IsTestableMethod.java:48)
	at org.junit.jupiter.engine.discovery.predicates.IsTestMethod.test(IsTestMethod.java:23)
	at org.junit.jupiter.engine.discovery.predicates.IsTestableMethod.test(IsTestableMethod.java:26)
	at java.base/java.util.function.Predicate.lambda$or$2(Predicate.java:101)
	at java.base/java.util.function.Predicate.lambda$or$2(Predicate.java:101)
	at org.junit.platform.commons.util.ReflectionUtils.findMethod(ReflectionUtils.java:1410)
	at org.junit.platform.commons.util.ReflectionUtils.isMethodPresent(ReflectionUtils.java:1306)
	at org.junit.jupiter.engine.discovery.predicates.IsTestClassWithTests.hasTestOrTestFactoryOrTestTemplateMethods(IsTestClassWithTests.java:50)
	at org.junit.jupiter.engine.discovery.predicates.IsTestClassWithTests.test(IsTestClassWithTests.java:46)
	at org.junit.jupiter.engine.discovery.ClassSelectorResolver.resolve(ClassSelectorResolver.java:67)
	at org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.lambda$resolve$2(EngineDiscoveryRequestResolution.java:135)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1602)
	at java.base/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:129)
	at java.base/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:527)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:513)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:150)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.findFirst(ReferencePipeline.java:647)
	at org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.resolve(EngineDiscoveryRequestResolution.java:189)
	at org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.resolve(EngineDiscoveryRequestResolution.java:126)
	at org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.resolveCompletely(EngineDiscoveryRequestResolution.java:92)
	... 33 more

matrei avatar Oct 18 '24 19:10 matrei

Shouldn't all the modules in grails-core.ROOT also be in the bom? Otherwise you can't do this for example: implementation 'org.grails:grails-web-url-mappings'

that was overlooked and they are now added. thanks

codeconsole avatar Oct 18 '24 19:10 codeconsole

org.junit.jupiter:junit-jupiter:api,engine:$junitVersion
org.junit.platform:junit-platform-runner::$junitPlatformRunnerVersion

removed

codeconsole avatar Oct 18 '24 19:10 codeconsole

Great! Almost there, but now the plugins and profiles don't come out right in the grails-bom pom.

    <plugins-cache/>
    <plugins-fields/>
    <plugins-geb/>
    <plugins-gorm.hibernate5/>
    <plugins-gorm.mongodb/>
    <plugins-gorm.neo4j/>
    <plugins-rxjava/>
    <plugins-rxjava2/>
    <plugins-scaffolding/>
    <plugins-views-json/>
    <plugins-views-json-templates/>
    <plugins-views-markup/>
    <profiles-angular/>
    <profiles-base/>
    <profiles-plugin/>
    <profiles-profile/>
    <profiles-react/>
    <profiles-rest-api/>
    <profiles-rest-api-plugin/>
    <profiles-vue/>
    <profiles-web/>
    <profiles-web-plugin/>

and

<dependency>
    <version>8.0.0-SNAPSHOT</version>
</dependency>
...

matrei avatar Oct 18 '24 19:10 matrei

Great! Almost there, but now the plugins and profiles don't come out right in the grails-bom pom.

@matrei cleaned up and fixed.

codeconsole avatar Oct 18 '24 20:10 codeconsole

cleaned up and fixed.

Now the .version suffix fell off the generated version property names (or maybe it was already like this, before the last commit) 😄

  <properties>
    <spring-boot.version>3.3.4</spring-boot.version>
    <ant>1.10.15</ant>
    <asciidoctorj>3.0.0</asciidoctorj>
    <aspectjrt>1.9.22.1</aspectjrt>
    <async>6.0.0-SNAPSHOT</async>
    <bson>5.1.4</bson>
    <byte-buddy>1.15.5</byte-buddy>
    <caffeine>3.1.8</caffeine>
    <commons-codec>1.17.1</commons-codec>
    <commons-text>1.12.0</commons-text>
    <converters>6.0.0-SNAPSHOT</converters>
    <directory-watcher>0.18.0</directory-watcher>
    <flying-saucer-pdf-openpdf>9.4.0</flying-saucer-pdf-openpdf>
    <grails-async>6.0.0-SNAPSHOT</grails-async>
    <grails-datastore>9.0.0-SNAPSHOT</grails-datastore>
    <grails-datastore-gorm-hibernate5>9.0.0-SNAPSHOT</grails-datastore-gorm-hibernate5>
    <grails-gdoc-engine>1.0.1</grails-gdoc-engine>
    <grails-gradle-plugin>7.0.0-SNAPSHOT</grails-gradle-plugin>
    <grails-testing-support>4.0.0-SNAPSHOT</grails-testing-support>
    <groovy>4.0.23</groovy>
    <gsp>7.0.0-SNAPSHOT</gsp>
    <h2>2.3.232</h2>
    <hamcrest-core>1.3</hamcrest-core>
    <jackson-databind>2.18.0</jackson-databind>
    <jakarta.annotation-api>3.0.0</jakarta.annotation-api>
    <jakarta.inject-api>2.0.1</jakarta.inject-api>
    <jakarta.persistence-api>3.1.0</jakarta.persistence-api>
    <jakarta.servlet-api>6.0.0</jakarta.servlet-api>
    <jakarta.xml.bind-api>4.0.2</jakarta.xml.bind-api>
    <jansi>2.4.1</jansi>
    <javaparser-core>3.26.2</javaparser-core>
    <jcl-over-slf4j>2.0.16</jcl-over-slf4j>
    <jline>2.14.6</jline>
    <jna>5.15.0</jna>
    <jsoup>1.18.1</jsoup>
    <mongodb-driver>5.1.4</mongodb-driver>
    <objenesis>3.4</objenesis>
    <plugins-cache>8.0.0-SNAPSHOT</plugins-cache>
    <plugins-fields>6.0.0-SNAPSHOT</plugins-fields>
    <plugins-geb>5.0.0-SNAPSHOT</plugins-geb>
    <plugins-gorm.hibernate5>9.0.0-SNAPSHOT</plugins-gorm.hibernate5>
    <plugins-gorm.mongodb>9.0.0-SNAPSHOT</plugins-gorm.mongodb>
    <plugins-gorm.neo4j>8.1.0</plugins-gorm.neo4j>
    <plugins-rxjava>1.1.1</plugins-rxjava>
    <plugins-rxjava2>2.0.0</plugins-rxjava2>
    <plugins-scaffolding>6.0.0-SNAPSHOT</plugins-scaffolding>
    <plugins-views-json>4.0.0-SNAPSHOT</plugins-views-json>
    <plugins-views-json-templates>4.0.0-SNAPSHOT</plugins-views-json-templates>
    <plugins-views-markup>4.0.0-SNAPSHOT</plugins-views-markup>
    <profiles-angular>10.0.1-SNAPSHOT</profiles-angular>
    <profiles-base>7.0.1-SNAPSHOT</profiles-base>
    <profiles-plugin>7.0.1-SNAPSHOT</profiles-plugin>
    <profiles-profile>7.0.1-SNAPSHOT</profiles-profile>
    <profiles-react>7.0.1-SNAPSHOT</profiles-react>
    <profiles-rest-api>7.0.1-SNAPSHOT</profiles-rest-api>
    <profiles-rest-api-plugin>7.0.1-SNAPSHOT</profiles-rest-api-plugin>
    <profiles-vue>7.0.1-SNAPSHOT</profiles-vue>
    <profiles-web>7.0.1-SNAPSHOT</profiles-web>
    <profiles-web-plugin>7.0.1-SNAPSHOT</profiles-web-plugin>
    <slf4j>2.0.16</slf4j>
    <snakeyaml>2.3</snakeyaml>
    <spock-core>2.3-groovy-4.0</spock-core>
    <spotbugs-annotations>4.8.6</spotbugs-annotations>
    <spring-boot-cli>3.3.4</spring-boot-cli>
    <springloaded>1.2.8.RELEASE</springloaded>
    <tomcat>10.1.31</tomcat>
    <tomcat-embed-logging-juli>8.5.2</tomcat-embed-logging-juli>
    <views-json-testing-support>4.0.0-SNAPSHOT</views-json-testing-support>
  </properties>

matrei avatar Oct 18 '24 20:10 matrei

Now the .version suffix fell off the generated version property names (or maybe it was already like this, before the last commit) 😄

@matrei thanks for pointing that out... didn't get much sleep last night and I am simultaneously trying to simplify and maintain code readability as I am introducing these fixes.

codeconsole avatar Oct 18 '24 21:10 codeconsole

@matrei anything else to add? have you looked at the dependabot solution?

codeconsole avatar Oct 19 '24 18:10 codeconsole

@codeconsole The versions don't come out right:

      <dependency>
        <groupId>org.grails</groupId>
        <artifactId>grails-bootstrap</artifactId>
        <version>${7.0.0-SNAPSHOT}</version>
      </dependency>

matrei avatar Oct 19 '24 18:10 matrei

have you looked at the dependabot solution?

I don't know, it seems to be a lot of moving parts here. There must be an easier way.

matrei avatar Oct 19 '24 19:10 matrei

have you looked at the dependabot solution?

I don't know, it seems to be a lot of moving parts here. There must be an easier way.

@matrei no moving parts. It was just a 1 time generation. The bom now uses the generated properties so if they are updated by dependabot they immediately take effect and re-generating would just generate the same thing that is already there. The generated dependabot build.gradle uses the generated properties and there is no need to update it to maintain existing versions and dependencies.

Regeneration would just show you if you are missing anything. How are we on everything else. Are we good to merge?

Lastly, the version names in gradle.properties are identical to the ones defined in the bom for consistency and to prevent confusion.

codeconsole avatar Oct 19 '24 19:10 codeconsole

Are we good to merge?

See earlier comment about versions not coming out right in the pom.

matrei avatar Oct 20 '24 09:10 matrei

I would like for one or more reviewer to also approve this PR before merging as it's so big and with such fundamental changes to the project.

Also, these are the only external dependencies not already present in the spring-boot bom. I believe the rest can be removed from grails-bom.

ant = '1.10.15'
asciidoctorj = '3.0.0'
commons-text = '1.12.0'
directory-watcher = '0.18.0'
flying-saucer-pdf-openpdf = '9.4.0'
jansi = '2.4.1'
javaparser = '3.26.2'
jline = '2.14.6'
jna = '5.15.0'
jsoup = '1.18.1'
objenesis = '3.4'
spock = '2.3-groovy-4.0'
spotbugs-annotations = '4.8.6'
spring-boot-cli = '3.3.4'
springloaded = '1.2.8.RELEASE'
tomcat-embed-logging-juli = '8.5.2'

The only fear here would be if the Spring Boot versions are not the same and not compatible. Perhaps the safes approach would be removing them later after verifying the current configuration does not introduce any issues. The current configuration matches what is currently present.

codeconsole avatar Oct 21 '24 01:10 codeconsole

I ran this against grails-functional-testing, https://github.com/grails/grails-functional-tests/tree/skip-4-failing-tests specifically and the only thing it needed was the version set for org.grails.plugins:hibernate5, which is incorrect in the bom as org.grails.plugins:gorm-hibernate.

jamesfredley avatar Oct 21 '24 17:10 jamesfredley

The few comments, plus I'd like to add these: org.gebish:geb-spock:7.0 com.bertramlabs.plugins:asset-pipeline-grails:5.0.1 org.grails.plugins:database-migration:6.0.0-SNAPSHOT

@jamesfredley comments resolved, dependencies added

codeconsole avatar Oct 21 '24 18:10 codeconsole

https://github.com/grails/grails-functional-tests/tree/skip-4-failing-tests passed all tests.

jamesfredley avatar Oct 21 '24 19:10 jamesfredley

@matrei if I new I was going to make so many changes, I would have separated the task names a long time ago when you first asked. lol.

🤣

codeconsole avatar Oct 21 '24 19:10 codeconsole

These are the version property names for the jakarta dependencies in spring-boot-dependencies that we duplicate. Should we try and keep the property names in sync or remove these dependences from grails-bom if we don't need to set different versions? Right now our property names are jakarta-*-api.version

<jakarta-annotation.version>2.1.1</jakarta-annotation.version>
<jakarta-inject.version>2.0.1</jakarta-inject.version>
<jakarta-persistence.version>3.1.0</jakarta-persistence.version>
<jakarta-servlet.version>6.0.0</jakarta-servlet.version>
<jakarta-xml-bind.version>4.0.2</jakarta-xml-bind.version>

As much as I would like to support patch updates. The right thing is prob to resolve to Spring.

removed...

matrei avatar Oct 21 '24 20:10 matrei

org.apache.groovy:groovy-groovydoc is in the bom and does not need the version specifed at https://github.com/grails/grails-core/blob/0342bd126723d9f77f88f715c3fdfa2a5a1c0d5f/gradle/docs.gradle#L23

matrei avatar Oct 22 '24 06:10 matrei