codeconsole commented on code in PR #16114:
URL: https://github.com/apache/grails-core/pull/16114#discussion_r3741917482
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy:
##########
@@ -204,33 +199,63 @@ class GrailsGradlePlugin implements Plugin<Project> {
if (grailsExtension != null) {
c.groovyOptions.forkOptions.jvmArgumentProviders.add(new
GrailsCompileStaticArtefactsProvider(grailsExtension.compileStatic))
}
- Closure<String> userScriptGenerator = getGroovyCompilerScript(c,
project)
- c.doFirst {
- // This isn't ideal - we're performing configuration at
execution time, but the alternative would be having
- // to maintain a clean / configuration task and then gradle
would want to cache those tasks. Since the inputs
- // to those tasks would effectively be the runtimeClasspath,
dependency problems can arise if another task
- // changes the runtimeClasspath. To prevent having to add
those tasks into the dependency chain, use doFirst
- File combinedFile = groovyCompilerConfigFile.get().asFile
- if (!combinedFile.exists()) {
- combinedFile.parentFile.mkdirs()
- combinedFile.createNewFile()
- }
+ }
- String configuredScript = null
- if (c.groovyOptions.configurationScript) {
- configuredScript =
c.groovyOptions.configurationScript.text?.trim() ?: null
- }
- String grailsScript = userScriptGenerator?.call()
+ // The combined compiler configuration script is produced by its own
task rather than from a
+ // doFirst on the compile task. Gradle finalizes task properties
before any task action runs,
+ // so assigning groovyOptions.configurationScript from doFirst fails
from Gradle 9.7 on, where
+ // GroovyCompileOptions became a lazy property — "The value for task
':compileGroovy' property
+ // 'groovyOptions.configurationScriptFile' is final and cannot be
changed any further." Once
+ // the property is assigned during configuration, Gradle also treats
the script as an input
+ // file that has to exist before the compile task runs, which a
producing task guarantees
+ // across a `clean build` and a doFirst cannot.
+ //
+ // Wiring happens after evaluation so a configurationScript set by the
build script is already
+ // in place and gets folded into the combined file rather than
clobbered. Names are read via
+ // TaskCollection.names, which does not realize the tasks.
+ project.afterEvaluate {
+ project.tasks.withType(GroovyCompile).names.each { String
compileTaskName ->
+ TaskProvider<GroovyCompile> compileTask =
project.tasks.named(compileTaskName, GroovyCompile)
+ // Use a task-specific config file to avoid overlapping
outputs when multiple
+ // GroovyCompile tasks exist in the same project (e.g.
compileGroovy, compileTestGroovy).
+ Provider<RegularFile> groovyCompilerConfigFile =
project.layout.buildDirectory.file("grailsGroovyCompilerConfig-${compileTaskName}.groovy")
+ File[] userConfigurationScript = new File[1]
+
+ TaskProvider<Task> generateGroovyCompilerConfig =
project.tasks.register("generate${compileTaskName.capitalize()}GrailsCompilerConfig")
{ Task t ->
+ t.description = "Generates the Grails Groovy compiler
configuration script for ${compileTaskName}"
+ t.outputs.file(groovyCompilerConfigFile)
+ // Generating the script needs the resolved compile
classpath, and declaring that as
+ // an input would pull the runtimeClasspath into this
task's up-to-date check. Since
+ // the inputs to this task would effectively be the
runtimeClasspath, dependency
+ // problems can arise if another task changes the
runtimeClasspath. Generating the
+ // script is cheap, so skip state tracking and regenerate
on every build instead.
+ t.doNotTrackState('Depends on the resolved compile
classpath; cheap to regenerate')
Review Comment:
Right that `doNotTrackState` establishes no ordering — the generator
declared no dependencies at all while its action resolves
`compileTask.classpath` and reads jar entries from it. Fixed in 8b8d063316: it
now depends on the classpath's own build dependencies.
I used `dependsOn` rather than declaring the classpath as an input, because
modelling it as an input reintroduces exactly what `doNotTrackState` is there
to prevent — it drags the runtimeClasspath into the up-to-date check.
`dependsOn` gets the ordering without the tracking.
For the record on severity: I could reproduce the missing edge, but not the
failure. In the natural task graph Gradle schedules the producing jar before
the generator, and the two probed classes normally come from external jars
already in the module cache. That matches your note that you had to force the
ordering. Still worth closing — it was a real latent hazard.
Test: `GrailsGroovyCompilerConfigSpec` — *the generator runs after the tasks
that produce the compile classpath*.
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy:
##########
@@ -204,33 +199,63 @@ class GrailsGradlePlugin implements Plugin<Project> {
if (grailsExtension != null) {
c.groovyOptions.forkOptions.jvmArgumentProviders.add(new
GrailsCompileStaticArtefactsProvider(grailsExtension.compileStatic))
}
- Closure<String> userScriptGenerator = getGroovyCompilerScript(c,
project)
- c.doFirst {
- // This isn't ideal - we're performing configuration at
execution time, but the alternative would be having
- // to maintain a clean / configuration task and then gradle
would want to cache those tasks. Since the inputs
- // to those tasks would effectively be the runtimeClasspath,
dependency problems can arise if another task
- // changes the runtimeClasspath. To prevent having to add
those tasks into the dependency chain, use doFirst
- File combinedFile = groovyCompilerConfigFile.get().asFile
- if (!combinedFile.exists()) {
- combinedFile.parentFile.mkdirs()
- combinedFile.createNewFile()
- }
+ }
- String configuredScript = null
- if (c.groovyOptions.configurationScript) {
- configuredScript =
c.groovyOptions.configurationScript.text?.trim() ?: null
- }
- String grailsScript = userScriptGenerator?.call()
+ // The combined compiler configuration script is produced by its own
task rather than from a
+ // doFirst on the compile task. Gradle finalizes task properties
before any task action runs,
+ // so assigning groovyOptions.configurationScript from doFirst fails
from Gradle 9.7 on, where
+ // GroovyCompileOptions became a lazy property — "The value for task
':compileGroovy' property
+ // 'groovyOptions.configurationScriptFile' is final and cannot be
changed any further." Once
+ // the property is assigned during configuration, Gradle also treats
the script as an input
+ // file that has to exist before the compile task runs, which a
producing task guarantees
+ // across a `clean build` and a doFirst cannot.
+ //
+ // Wiring happens after evaluation so a configurationScript set by the
build script is already
+ // in place and gets folded into the combined file rather than
clobbered. Names are read via
+ // TaskCollection.names, which does not realize the tasks.
+ project.afterEvaluate {
+ project.tasks.withType(GroovyCompile).names.each { String
compileTaskName ->
+ TaskProvider<GroovyCompile> compileTask =
project.tasks.named(compileTaskName, GroovyCompile)
+ // Use a task-specific config file to avoid overlapping
outputs when multiple
+ // GroovyCompile tasks exist in the same project (e.g.
compileGroovy, compileTestGroovy).
+ Provider<RegularFile> groovyCompilerConfigFile =
project.layout.buildDirectory.file("grailsGroovyCompilerConfig-${compileTaskName}.groovy")
+ File[] userConfigurationScript = new File[1]
+
+ TaskProvider<Task> generateGroovyCompilerConfig =
project.tasks.register("generate${compileTaskName.capitalize()}GrailsCompilerConfig")
{ Task t ->
+ t.description = "Generates the Grails Groovy compiler
configuration script for ${compileTaskName}"
+ t.outputs.file(groovyCompilerConfigFile)
+ // Generating the script needs the resolved compile
classpath, and declaring that as
+ // an input would pull the runtimeClasspath into this
task's up-to-date check. Since
+ // the inputs to this task would effectively be the
runtimeClasspath, dependency
+ // problems can arise if another task changes the
runtimeClasspath. Generating the
+ // script is cheap, so skip state tracking and regenerate
on every build instead.
+ t.doNotTrackState('Depends on the resolved compile
classpath; cheap to regenerate')
+ t.doLast {
+ File combinedFile =
groovyCompilerConfigFile.get().asFile
+ combinedFile.parentFile.mkdirs()
+
+ String configuredScript = null
+ if (userConfigurationScript[0]?.exists()) {
+ configuredScript =
userConfigurationScript[0].text?.trim() ?: null
+ }
+ String grailsScript =
getGroovyCompilerScript(compileTask.get(), project)?.call()
+
+ String combinedScripts = """
+ // Grails groovy compilation configuration to
ensure ASTs are applied correctly
+
+ ${grailsScript?.trim() ?: ''}
- String combinedScripts = """
- // Grails groovy compilation configuration to ensure ASTs
are applied correctly
-
- ${grailsScript?.trim() ?: ''}
+ ${configuredScript?.trim() ?: ''}
+ """
+ combinedFile.write(combinedScripts)
+ }
+ }
- ${configuredScript?.trim() ?: ''}
- """
- combinedFile.write(combinedScripts)
- c.groovyOptions.configurationScript = combinedFile
+ compileTask.configure { GroovyCompile c ->
+ userConfigurationScript[0] =
c.groovyOptions.configurationScript
Review Comment:
Confirmed and fixed in 8b8d063316. Capturing during `afterEvaluate` meant an
assignment from any later callback simply overwrote the combined file and the
Grails imports disappeared with no error.
Capture and assignment now happen at `taskGraph.whenReady` — the last point
before execution and after every configuration callback has run, so the user's
final value is what gets folded in. The property is still assignable there on
9.7.
Scope note for anyone reading later: the ordinary path — assigning
`configurationScript` directly in the build script — was already merging
correctly. Only assignment from a later callback was affected.
Test: `GrailsGroovyCompilerConfigSpec` — *a configurationScript assigned
from a later callback is folded in, not clobbered*.
--
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]
