codeconsole commented on PR #16114:
URL: https://github.com/apache/grails-core/pull/16114#issuecomment-5289016409
Both questions landed on something real, and chasing the second one led back
to the first. The
answer to both turned out to be to stop reading the classpath at all.
## Why the classpath was involved
`isClassOnClasspath` opened jars on the compile classpath to decide whether
to add the
`grails.gorm.annotation` and `grails.plugin.scaffolding.annotation` star
imports. That one probe
forced everything else: the script could only be built at execution time,
which is why it was
computed in a task action, why the task opted out of state tracking, and why
it needed an ordering
edge to whatever produced the classpath.
The probe bought nothing. A star import of a package that is not on the
classpath contributes no
classes and is not an error in Groovy — verified on 5.0.8 for both dynamic
and `@CompileStatic`
compilation. `jakarta.validation.constraints` in the same block was already
unconditional; the two
probed packages were the outlier. They are unconditional now too.
That is the one behavioural change here. Given this is 8.0.x it seemed the
right moment, but it is
easy to put back if you would rather keep the probe.
## On the design you reverted in 9a529d1996
Your concern was well founded and worth being precise about.
That design declared the runtime classpath as a **task input**:
```groovy
task.inputs.files(project.configurations.named('runtimeClasspath'))
```
An input has to be snapshotted, so `runtimeClasspath` was resolved for the
up-to-date check and
became part of the fingerprint — which is where "task dependency errors if
the runtimeclasspath is
changed" comes from.
The generator now reads no classpath at all. It takes the finished script as
a single `@Input` and
writes it:
```
GENERATOR_DEPENDS_ON_COMPILE_CLASSPATH=false
GENERATOR_DEPENDS_ON_RUNTIME_CLASSPATH=false
GENERATOR_DECLARED_INPUT_FILES=0
```
So no configuration enters its dependency chain or its up-to-date check. The
hazard you removed is
structurally absent rather than narrowly avoided, and the task is properly
tracked rather than
`doNotTrackState` — an unchanged build is `UP-TO-DATE`, changing the imports
regenerates.
Separately, and not caused by this PR: mutating the runtime classpath from a
late callback still
fails in a Grails app today, with
```
Cannot mutate the dependencies of configuration ':runtimeOnly' after the
configuration's
child configuration ':grailsCliDetect' was resolved.
```
That reproduces identically on `8.0.x` without this branch — it comes from
the CLI companion probe,
not from the compiler config wiring. Happy to raise it separately if that is
news.
## On whether Gradle's restriction is artificial
I don't think a ticket would go anywhere, because Gradle shipped the
replacement rather than closing
the door. `GroovyCompileOptions.getConfigurationScript()` is annotated
`@ReplacedBy("configurationScriptFile")`, and `getConfigurationScriptFile()`
returns a
`RegularFileProperty`. Lazy task properties are finalized before execution
so inputs can be
fingerprinted for up-to-date checks and the configuration cache; that is the
point of the migration,
not an incidental restriction. The branch now uses that property.
The `doFirst` was also quietly wrong before 9.7. It declared the script as
an **output** of the
compile task and assigned it after input snapshotting, so the script never
took part in
`compileGroovy`'s up-to-date check — change what the script generated and
the compile task would not
re-run.
## A regression this turned up
Worth flagging on its own.
`GrailsPluginGradlePlugin.getGroovyCompilerScript` registers the project
version and name as inputs of the compile task, because it bakes them into
compiled classes as AST
metadata. When the script computation moved into a task action, those
`inputs.property` calls ran too
late to take effect:
| commit | `compileGroovy` has a `version` input |
|---|---|
| 3e4fa5c897 (before this PR) | true |
| 79504824fd (this PR as first pushed) | **false** |
| now | true |
So as first pushed, changing a plugin's version would have left the stamped
metadata stale. Building
the script during configuration restores it, and there is a test for it.
## Where it ended up
The generator is a `GrailsCompilerConfigScriptTask` with an `@Input
Property<String>` and an
`@OutputFile`. It reads no classpath, declares no file inputs, is up-to-date
checked, and stores and
reuses a configuration cache entry. It waits for a task that produces the
build's own
`configurationScript`, if there is one — without that edge the build's
script was silently dropped on
a clean build and only appeared from the second build onwards.
Seven tests in `GrailsGroovyCompilerConfigSpec` cover the lifecycle
properties. `:grails-gradle:test`,
`codeStyle` and a full root `build -PskipTests` are green on 9.7.0.
--
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]
