jdaugherty commented on code in PR #15365:
URL: https://github.com/apache/grails-core/pull/15365#discussion_r3141966343
##########
dependencies.gradle:
##########
@@ -73,20 +75,26 @@ ext {
'asset-pipeline-bom.version' : '5.1.0-M4',
'bootstrap-icons.version' : '1.13.1',
'bootstrap.version' : '5.3.8',
- 'commons-codec.version' : '1.19.0',
+ 'commons-codec.version' : '1.21.0',
'commons-lang3.version' : '3.20.0',
'geb-spock.version' : '8.0.1',
'groovy.version' : '4.0.31',
'jquery.version' : '3.7.1',
'liquibase-hibernate5.version': '4.27.0',
'mongodb.version' : '5.6.4',
+ // Selenium 4.41+ requires OpenTelemetry 1.59+, which is newer
than the
+ // 1.55.x that Spring Boot 4.0.5 manages. Pin the version on the
individual
+ // OpenTelemetry modules (see the constraints block in
grails-bom/build.gradle)
+ // so the explicit constraint overrides whatever Spring Boot's
imported BOM
+ // declares, regardless of platform import ordering.
+ 'opentelemetry.version' : '1.59.0',
'rxjava.version' : '1.3.8',
'rxjava2.version' : '2.2.21',
'rxjava3.version' : '3.1.12',
- 'selenium.version' : '4.38.0',
+ 'selenium.version' : '4.41.0',
Review Comment:
We had pinned selenium to .38 due to the geb issues. Do we know if these
are resolved?
##########
settings.gradle:
##########
@@ -20,6 +20,23 @@ import org.apache.tools.ant.DirectoryScanner
import org.gradle.api.initialization.resolve.RepositoriesMode
pluginManagement {
+ repositories {
Review Comment:
We should use https://github.com/apache/grails-core/pull/15597 instead
##########
dependencies.gradle:
##########
@@ -22,15 +22,16 @@
// These files are split to facilitate separation of build vs application
dependencies. These are the application dependencies.
ext {
gradleBomDependencyVersions = [
- 'ant.version' : '1.10.15',
+ 'ant.version' : '1.10.17',
'asciidoctor-gradle-jvm.version': '4.0.5',
'asciidoctorj.version' : '3.0.0',
'asset-pipeline-gradle.version' : '5.1.0-M4',
- 'byte-buddy.version' : '1.17.8',
- 'commons-text.version' : '1.14.0',
+ 'byte-buddy.version' : '1.18.8',
+ 'commons-text.version' : '1.15.0',
'directory-watcher.version' : '0.19.1',
- 'gradle-spock.version' : '2.3-groovy-3.0',
- 'grails-publish-plugin.version' : '0.0.4',
+ 'gradle-groovy.version' : '4.0.31',
Review Comment:
This should be GroovySystem version instead of hard coding.
##########
buildSrc/settings.gradle:
##########
@@ -17,6 +17,23 @@
* under the License.
*/
pluginManagement {
+ repositories {
+ mavenCentral()
+ gradlePluginPortal()
+ // Needed to resolve grails-publish snapshots that the build-logic
composite
Review Comment:
We should take https://github.com/apache/grails-core/pull/15597 instead of
these changes.
##########
grails-gradle/buildSrc/settings.gradle:
##########
@@ -17,6 +17,23 @@
* under the License.
*/
pluginManagement {
+ repositories {
Review Comment:
We should use https://github.com/apache/grails-core/pull/15597 instead
##########
gradle.properties:
##########
@@ -50,8 +50,8 @@ yakworksHibernateGroovyProxyVersion=1.1
# Build dependency versions not managed by BOMs
apacheRatVersion=0.8.1
gradleChecksumPluginVersion=1.4.0
-# note: the cyclonedx 3 requires Gradle 9
-gradleCycloneDxPluginVersion=2.4.1
+# CycloneDX Gradle plugin version
Review Comment:
Remove the comment
##########
grails-forge/buildSrc/settings.gradle:
##########
@@ -17,6 +17,23 @@
* under the License.
*/
pluginManagement {
+ repositories {
Review Comment:
We should use https://github.com/apache/grails-core/pull/15597 instead
##########
dependencies.gradle:
##########
@@ -73,20 +75,26 @@ ext {
'asset-pipeline-bom.version' : '5.1.0-M4',
'bootstrap-icons.version' : '1.13.1',
'bootstrap.version' : '5.3.8',
- 'commons-codec.version' : '1.19.0',
+ 'commons-codec.version' : '1.21.0',
'commons-lang3.version' : '3.20.0',
'geb-spock.version' : '8.0.1',
'groovy.version' : '4.0.31',
'jquery.version' : '3.7.1',
'liquibase-hibernate5.version': '4.27.0',
'mongodb.version' : '5.6.4',
+ // Selenium 4.41+ requires OpenTelemetry 1.59+, which is newer
than the
Review Comment:
Is this upgrade just to prevent conflicts with forge's version of micronaut?
##########
grails-forge/settings.gradle:
##########
@@ -19,6 +19,23 @@
import org.gradle.api.initialization.resolve.RepositoriesMode
pluginManagement {
+ repositories {
Review Comment:
We should use https://github.com/apache/grails-core/pull/15597 instead
##########
grails-forge/grails-forge-api/build.gradle:
##########
@@ -47,8 +48,8 @@ dependencies {
api 'io.micronaut:micronaut-http-client'
api 'io.micronaut:micronaut-inject'
api 'io.micronaut:micronaut-runtime'
- api 'io.micronaut:micronaut-validation'
- api group: 'javax.inject', name: 'javax.inject', version: '1'
+ api 'io.micronaut.validation:micronaut-validation'
+ api "jakarta.inject:jakarta.inject-api:$jakartaInjectVersion"
Review Comment:
So the jakarta api is defined in the micronaut platform, but not the inject
version. Is inject included transitively by the bom dependencies? I'm worried
about mismatching this version vs what's in their bom.
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy:
##########
@@ -968,12 +975,17 @@ ${importStatements}
@CompileDynamic
protected TaskProvider<Task> createNative2AsciiTask(TaskContainer tasks,
src, dest) {
TaskProvider<Task> native2asciiTask =
tasks.register('native2ascii').configure {
+ it.inputs.dir(src)
+ it.outputs.dir(dest)
+
+ // Capture ant builder at configuration time to avoid Task.project
access at execution time
Review Comment:
Let's remove the comment, it's obvious once we're on Gradle 9 why we did
this.
##########
grails-bom/build.gradle:
##########
@@ -107,6 +107,8 @@ configurations.register('bomDependencies').configure {
}
tasks.register('extractConstraints', ExtractDependenciesTask).configure {
ExtractDependenciesTask it ->
+ // Capture project services at configuration time so the task avoids the
deprecated Task.project at execution time
Review Comment:
This comment is "this is bridge" , let's remove it.
##########
grails-gradle/settings.gradle:
##########
@@ -19,14 +19,31 @@
import org.gradle.api.initialization.resolve.RepositoriesMode
pluginManagement {
+ repositories {
Review Comment:
We should use https://github.com/apache/grails-core/pull/15597 instead
##########
grails-forge/test-core/build.gradle:
##########
@@ -59,15 +59,15 @@ dependencies {
testCompileOnly
"io.micronaut:micronaut-inject-groovy:$micronautVersion"
}
- testImplementation "org.codehaus.groovy:groovy:$groovyVersion"
+ testImplementation "org.apache.groovy:groovy:$groovyVersion"
Review Comment:
This doesn't look fixed, the groovyVersion is still being reference. The
micronaut bom includes `groovy-bom` so the versions should be unneccessary in
forge unless there is a version mismatch. Also, since the recent patches of
groovy, we know it's important to use the same version. I was proposing we
drop the property groovyVersion and use the micronaut bom instead.
##########
grails-forge/grails-forge-analytics-postgres/build.gradle:
##########
@@ -18,7 +18,6 @@
*/
plugins {
- id 'com.gradleup.shadow'
id 'org.apache.grails.buildsrc.properties'
Review Comment:
The shadow plugin is used to create a fat jar, why was it removed?
##########
grails-test-examples/plugins/issue-11767/build.gradle:
##########
@@ -45,3 +41,61 @@ apply {
from
rootProject.layout.projectDirectory.file('gradle/functional-test-config.gradle')
from
rootProject.layout.projectDirectory.file('gradle/grails-extension-gradle-config.gradle')
}
+
+// Root-cause fix for duplicate META-INF/spring-configuration-metadata.json:
+//
+// This plugin intentionally exposes both a Java bean
(PluginJavaMicronautBean) and a
+// Groovy bean (PluginGroovyMicronautBean) that share the
@ConfigurationProperties('my')
+// prefix to exercise the original issue 11767 scenario. As a side effect, two
different
+// Micronaut tools produce the same metadata file:
+// - micronaut-inject-java runs as a Java annotation processor during
compileJava and
+// writes
build/classes/java/main/META-INF/spring-configuration-metadata.json
+// describing PluginJavaMicronautBean.
+// - micronaut-inject-groovy (pulled in transitively via grails-micronaut)
runs as a
+// Groovy AST transform during compileGroovy and writes
+// build/classes/groovy/main/META-INF/spring-configuration-metadata.json
describing
+// PluginGroovyMicronautBean.
+// The two files are not equivalent: each describes the beans visible to its
own compiler.
+// The shared CompilePlugin convention sets jar.duplicatesStrategy = FAIL so
the build
+// surfaces any double-configuration. Here the duplicate is by design, so we
merge the
+// two JSON documents into a single file (preserving every group / property /
hint) and
+// remove the now-merged source so the jar task sees exactly one metadata
file. This is
+// preferable to dropping one of the documents (DuplicatesStrategy.EXCLUDE)
because that
+// would silently lose the bean type information for one of the languages.
+def mergeMicronautConfigMetadata =
tasks.register('mergeMicronautConfigMetadata') {
+ description = 'Merges the spring-configuration-metadata.json emitted
separately by ' +
+ 'micronaut-inject-java and micronaut-inject-groovy into one
combined document.'
+ group = 'build'
+
+ def metadataRelativePath = 'META-INF/spring-configuration-metadata.json'
+ def javaMetadataProvider =
layout.buildDirectory.file("classes/java/main/${metadataRelativePath}")
+ def groovyMetadataProvider =
layout.buildDirectory.file("classes/groovy/main/${metadataRelativePath}")
+
+ // Re-run whenever either compile step emits new metadata; the task
mutates files
+ // inside the compile output directories so we cannot rely on standard
up-to-date
+ // tracking. The work is cheap (two small JSON files) so always running is
acceptable.
+ outputs.upToDateWhen { false }
Review Comment:
This is the incorrect way to do this. we should be able to cache these
merges, otherwise it will always run this in our build.
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/profiles/GrailsProfileGradlePlugin.groovy:
##########
@@ -87,9 +87,14 @@ class GrailsProfileGradlePlugin implements Plugin<Project> {
project.configurations.named('runtimeElements')
.configure { it.extendsFrom(runtimeOnlyConfiguration.get()) }
- TaskProvider<Task> processProfileResourcesTask =
project.tasks.register('processProfileResources')
- processProfileResourcesTask.configure { Task task ->
+ // Use Sync task type instead of project.sync in doLast to avoid
Task.project access at execution time
Review Comment:
Let's remove the comment, it's unnecessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
