jdaugherty commented on code in PR #15365:
URL: https://github.com/apache/grails-core/pull/15365#discussion_r3094018532
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/PublishPlugin.groovy:
##########
@@ -92,9 +92,14 @@ class PublishPlugin implements Plugin<Project> {
task.group = 'publishing'
task.outputs.dir(artifactsDir)
task.dependsOn(project.tasks.withType(Jar))
+
+ // Capture publishing extension at configuration time to avoid
Task.project access at execution time
+ // See:
https://docs.gradle.org/current/userguide/configuration_cache.html#config_cache:requirements:use_project_during_execution
+ def publishingExtension =
project.extensions.getByType(PublishingExtension)
+
task.doLast {
Map<String, String> artifacts = [:]
-
project.extensions.getByType(PublishingExtension).publications.withType(MavenPublication).each
{ MavenPublication publication ->
+
publishingExtension.publications.withType(MavenPublication).each {
MavenPublication publication ->
Review Comment:
We use the publish plugin and it definitely needs updated to support Gradle
9, but I don't see any PR's on https://github.com/apache/grails-gradle-publish
Shouldn't we update this plugin prior to merging this PR since it's also
incompatible with 9?
##########
gradle.properties:
##########
@@ -61,8 +61,8 @@ ersatzVersion=4.0.1
grailsSpringSecurityVersion=7.0.2-SNAPSHOT
jbossTransactionApiVersion=2.0.0.Final
# Note: we do not import the micronaut bom in our tests to avoid spring
version mismatches
-micronautHttpClientVersion=4.9.9
-micronautSerdeJacksonVersion=2.11.0
+micronautHttpClientVersion=4.10.18
Review Comment:
Can we just update the test example that uses these 2 micronaut values to
use the micronaut bom & not refer to specific versions? Mattias already
removed all other usages other than the test app for micronaut.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/SbomPlugin.groovy:
##########
@@ -145,13 +145,11 @@ class SbomPlugin implements Plugin<Project> {
}
private static void configureSbomTask(Project project,
Provider<RegularFile> sbomOutputLocation) {
- project.tasks.withType(CycloneDxTask).configureEach { CycloneDxTask
task ->
+ project.tasks.withType(CyclonedxDirectTask).configureEach {
CyclonedxDirectTask task ->
Review Comment:
Have you compared the new vs old task output and confirmed our doLast hack
still works?
##########
build-logic/docs-core/src/main/groovy/org/apache/grails/gradle/tasks/bom/ExtractDependenciesTask.groovy:
##########
@@ -83,17 +86,26 @@ abstract class ExtractDependenciesTask extends DefaultTask {
@Input
abstract MapProperty<String, String> getProjectCoordinateProperties()
+ // Captured at configuration time to avoid deprecated Task.project access
at execution time.
+ // See:
https://docs.gradle.org/current/userguide/configuration_cache.html#config_cache:requirements:use_project_during_execution
+ @Internal
+ DependencyHandler dependencyHandler
+
+ @Internal
+ ConfigurationContainer configurationContainer
+
void setConfiguration(NamedDomainObjectProvider<Configuration> config) {
dependencyArtifacts.from(config)
configurationName.set(config.name)
}
- ExtractDependenciesTask() {
- doFirst {
- if (!project.pluginManager.hasPlugin('java-platform')) {
Review Comment:
I agree this is the right short term solution, but this task will ship
publicly again. I was speaking to Vampire in Gradle chat, eventually I'll
probably add this is as it's own gradle plugin where the doFirst { } throws an
error. Then the plugin removes the action so it passes. Something like this
(from Vampire's chat):
val foo = Action<Task> {
error("FOO")
}
tasks.help {
doFirst("foo", foo)
}
pluginManager.withPlugin("java-platform") {
tasks.help {
actions.remove(foo)
}
}
But before doing any of this, did you see if you can store a reference to
the pluginManager and still have this validation? Is that allowed?
##########
grails-doc/src/en/ref/Configuration/Application Properties.adoc:
##########
@@ -0,0 +1,866 @@
+////
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+////
+
+== Application Properties
+
+A comprehensive reference of all configuration properties specific to Grails
and its bundled modules. These properties are set in
`grails-app/conf/application.yml` (or `application.groovy`).
Review Comment:
I dont' think this change should be made as part of micronaut PR
##########
grails-forge/gradle.properties:
##########
@@ -23,27 +23,28 @@
title=Grails Application Forge
projectDesc=Generates Grails applications
-# for forge build process which is a micronaut 3.x.x app with picocli
+# for forge build process which is a micronaut 4.x.x app with picocli
grailsPublishGradleVersion=0.0.2
antVersion=1.10.15
antlr4Version=4.8-1!!
-asciidoctorGradleJvmVersion=4.0.4
+asciidoctorGradleJvmVersion=4.0.5
cglibVersion=3.3.0
+byteBuddyVersion=1.15.11
commonsCompressVersion=1.27.1
gradleSdkvendorPluginVersion=3.0.0
-groovyVersion=3.0.25
+groovyVersion=4.0.30
Review Comment:
Maybe bump to 4.0.31 to match grails-core?
##########
grails-bom/build.gradle:
##########
@@ -107,6 +107,11 @@ 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
+ it.captureProjectServices(project.dependencies, project.configurations)
+ if (!project.pluginManager.hasPlugin('java-platform')) {
Review Comment:
The only reason this existed before was in case external parties used our
task. Let's remove it from here.
##########
grails-forge/grails-forge-api/build.gradle:
##########
@@ -57,22 +58,23 @@ dependencies {
testImplementation 'io.micronaut:micronaut-http-client'
testImplementation 'io.micronaut:micronaut-http-server-netty'
testImplementation 'io.micronaut.test:micronaut-test-spock'
+ testRuntimeOnly 'io.micronaut:micronaut-jackson-databind'
testCompileOnly "io.micronaut:micronaut-inject-groovy:$micronautVersion"
Review Comment:
We have the micronaut bom, why keep specifying values individually? Can't
we adopt their bom and drop custom versions?
##########
grails-doc/build.gradle:
##########
@@ -306,12 +306,13 @@ docsTask.configure { Sync it ->
}
tasks.register('dist', Zip).configure { Zip it ->
- it.dependsOn(docsTask)
+ it.dependsOn(docsTask, tasks.named('groovydoc'))
Review Comment:
Why is groovydoc a requirement of dist? Isn't it part of the build already?
##########
grails-forge/grails-forge-web-netty/build.gradle:
##########
@@ -32,6 +31,7 @@ dependencies {
implementation 'io.micronaut.gcp:micronaut-gcp-http-client'
runtimeOnly 'ch.qos.logback:logback-classic'
+ runtimeOnly 'io.micronaut:micronaut-jackson-databind'
Review Comment:
I thought we used serde instead of jackson?
##########
build-logic/docs-core/src/main/groovy/org/apache/grails/gradle/tasks/bom/ExtractDependenciesTask.groovy:
##########
@@ -83,17 +86,26 @@ abstract class ExtractDependenciesTask extends DefaultTask {
@Input
abstract MapProperty<String, String> getProjectCoordinateProperties()
+ // Captured at configuration time to avoid deprecated Task.project access
at execution time.
+ // See:
https://docs.gradle.org/current/userguide/configuration_cache.html#config_cache:requirements:use_project_during_execution
+ @Internal
+ DependencyHandler dependencyHandler
+
+ @Internal
+ ConfigurationContainer configurationContainer
+
void setConfiguration(NamedDomainObjectProvider<Configuration> config) {
dependencyArtifacts.from(config)
configurationName.set(config.name)
}
- ExtractDependenciesTask() {
- doFirst {
- if (!project.pluginManager.hasPlugin('java-platform')) {
Review Comment:
AI seems to agree:
public abstract class MyTask extends DefaultTask {
@Input
public abstract Property<Boolean> getRequiredPluginApplied();
@TaskAction
public void run() {
if (!getRequiredPluginApplied().get()) {
throw new GradleException(
"Task '" + getPath() + "' requires plugin
'com.example.required' to be applied.");
}
// real task work
}
}
public class MyPlugin implements Plugin<Project> {
@Override
public void apply(Project project) {
TaskProvider<MyTask> myTask =
project.getTasks().register("myTask", MyTask.class, task -> {
task.getRequiredPluginApplied().convention(false);
});
project.getPluginManager().withPlugin("com.example.required", applied ->
myTask.configure(task ->
task.getRequiredPluginApplied().set(true))
);
}
}
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsPluginGradlePlugin.groovy:
##########
@@ -248,19 +249,17 @@ class GrailsPluginGradlePlugin extends GrailsGradlePlugin
{
project.afterEvaluate() {
ProcessResources processResources = (ProcessResources)
project.tasks.getByName('processResources')
- def processResourcesDependencies = []
-
- processResourcesDependencies << project.task(type: Copy,
'copyCommands') {
+ TaskProvider<Copy> copyCommands =
project.tasks.register('copyCommands', Copy) {
from("${project.projectDir}/src/main/scripts")
into("${processResources.destinationDir}/META-INF/commands")
}
- processResourcesDependencies << project.task(type: Copy,
'copyTemplates') {
+ TaskProvider<Copy> copyTemplates =
project.tasks.register('copyTemplates', Copy) {
from("${project.projectDir}/src/main/templates")
into("${processResources.destinationDir}/META-INF/templates")
}
processResources.setDuplicatesStrategy(DuplicatesStrategy.INCLUDE)
- processResources.dependsOn(*processResourcesDependencies)
+ processResources.dependsOn(copyCommands, copyTemplates)
Review Comment:
Was this an intentional change or an AI changing the style?
##########
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:
Can't we use the micronaut bom?
##########
grails-forge/grails-forge-core/build.gradle:
##########
@@ -41,13 +41,15 @@ sourceSets {
}
dependencies {
- annotationProcessor
platform("io.micronaut:micronaut-bom:$micronautVersion")
- implementation platform("io.micronaut:micronaut-bom:$micronautVersion")
+ annotationProcessor
platform("io.micronaut.platform:micronaut-platform:$micronautVersion")
Review Comment:
Can't we adopt the micronaut bom to set the versions below?
##########
grails-forge/grails-forge-cli/build.gradle:
##########
@@ -68,10 +68,10 @@ ext {
}
dependencies {
- annotationProcessor
platform("io.micronaut:micronaut-bom:$micronautVersion")
- implementation platform("io.micronaut:micronaut-bom:$micronautVersion")
+ annotationProcessor
platform("io.micronaut.platform:micronaut-platform:$micronautVersion")
+ implementation
platform("io.micronaut.platform:micronaut-platform:$micronautVersion")
Review Comment:
Can't we adopt the micronaut bom to set the versions below?
--
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]
