Groovy 5.0.x support for Grails 8 + Spring Boot 4#15557
Conversation
This reverts commit 457d6cd.
# Conflicts: # build.gradle # dependencies.gradle # grails-forge/build.gradle # grails-gradle/build.gradle
# Conflicts: # buildSrc/build.gradle # dependencies.gradle # grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy # grails-gradle/buildSrc/build.gradle
# Conflicts: # dependencies.gradle # gradle/test-config.gradle # grails-forge/settings.gradle # settings.gradle
# Conflicts: # gradle.properties # grails-core/src/test/groovy/org/grails/plugins/BinaryPluginSpec.groovy
… + latest Jackson)
Cherry-picked comprehensive Groovy 5 compat from 9574fe8. Conflict resolutions: - dependencies.gradle: Groovy 5.0.5 GA (not SNAPSHOT) + Jackson 2.21.2 - LoggingTransformer: Keep manual log field injection (avoids Groovy 5 VariableScopeVisitor NPE entirely) - TransactionalTransformSpec: Remove direct Spock feature method invocation (Groovy 5/Spock 2.x incompatible) - grails-test-core/build.gradle: Remove spock-core transitive=false, keep junit-platform-suite - grails-test-suite-uber/build.gradle: Remove spock-core transitive=false and explicit byte-buddy
jdaugherty
left a comment
jdaugherty
left a comment
There was a problem hiding this comment.
I think we could move forward with this if we open subsequent tickets on the identified issues. I would really like to hear from @paulk-asert on the static compilation workaround in gsp & on the ? vs object assignment issue
@jamesfredley I believe we can also remove the inlines now?
| uses: github/codeql-action/init@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0 | ||
| with: | ||
| languages: ${{ matrix.language }} | ||
| # The autobuild action runs `./gradlew testClasses`, which compiles Groovy sample apps that are covered by regular CI. |
There was a problem hiding this comment.
Don't we need to compile it? Isn't setting this to non not building for it?
| private void handleArgumentClosure(CustomArgument argument, @DelegatesTo(strategy = Closure.DELEGATE_ONLY)Closure closure) { | ||
| withDelegate(closure, (Object)argument) | ||
| // Inlined ExecutesClosures.withDelegate: Groovy 5 STC can't resolve a parent trait's static method. | ||
| if (closure != null) { |
There was a problem hiding this comment.
Isn't this fixed now so the inline is unnecessary?
| field.nullable(defaultNull) | ||
| withDelegate(closure, (Object)field) | ||
| // Inlined ExecutesClosures.withDelegate: Groovy 5 STC can't resolve a parent trait's static method. | ||
| if (closure != null) { |
There was a problem hiding this comment.
I thought this is fixed and the inline is unnecessary?
|
|
||
| trait TestTrait<F extends Serializable> { | ||
| F from | ||
| trait TestTrait<T> { |
There was a problem hiding this comment.
This code is still here but I thought @matrei opened a ticket and its been fixed
# Conflicts: # dependencies.gradle
…2041 Per review feedback, switch the two GspCompileStaticSpec negative (undeclared-variable) specs from @IgnoreIf to @PendingFeatureIf so they fail - prompting removal of both the guards and the GroovyPageTypeCheckingExtension methodNotFound name-matching workaround - the moment GROOVY-12041 (open) is fixed and the undeclared-variable check is reported again. On Groovy 5 the two specs report as pending (skipped); on Groovy 4 they pass unchanged. Assisted-by: claude-code:claude-4.8-opus
GROOVY-12091 (bounded generic trait property setter remains abstract in the implementing class) is Resolved/Fixed for Groovy 5.0.7 and verified on the consumed 5.0.7-SNAPSHOT, so ClassPropertyFetcherTests.TestTrait restores its original `<F extends Serializable>` bound and drops the Groovy 5 workaround. Verified by ClassPropertyFetcherTests.testClassPropertyFetcherWithTraitProperty. Assisted-by: Sisyphus:anthropic/claude-opus-4-8
|
Burned down the Groovy 5 workarounds against the current Removed (now fixed upstream):
Checked, still required (kept):
The PR description at the top has been updated to reflect the current state. |
…Groovy 5 The compileStaticTagLibs support merged from 8.0.x (ControllerTagLibTypeCheckingExtension) relied on the unresolvedVariable / unresolvedProperty callbacks firing for the namespace dispatcher receiver (e.g. the `cst` in `cst.greet(...)`). On Groovy 5 those callbacks no longer fire when the controller inherits getProperty(String) (GROOVY-12041), so the dispatcher resolves to Object and the subsequent tag call failed static type checking with "Cannot find matching method java.lang.Object#<tag>(...)". methodNotFound now recognises that case directly: when the receiver is Object-typed and the object expression is a bare-variable or this-property dispatcher, the call is made dynamic - matching this extension's existing intent to silence namespace dispatch in controllers. Calls on declared fields/locals keep their concrete receiver type and remain fully type-checked, so the type-safety guarantees are preserved. This fixes the Groovy 5 compile failures in ControllerCompileStaticTagLibSpec (grails-gsp) and the demo33 CompileStaticController, which were cascading into the full build, functional, mongodb, hibernate and SiteMesh CI lanes. Assisted-by: Sisyphus:anthropic/claude-opus-4-8
Assisted-by: opencode:gpt-5.5
|
@jamesfredley your last update said you were going to file a dedicated ticket - has this been done? |
|
@jdaugherty I will try to get it filed today. Have had a small amount of time. And should have a commit to fix all the breakage in 1-2 hours after h7.4 was merged in. |
|
@jamesfredley did you see the GSP workaround that Paul posted as well on the groovy ticket? That would resolve the gsp issue. |
Assisted-by: hephaestus:gpt-5.5
|
@jdaugherty https://issues.apache.org/jira/browse/GROOVY-12106 and I will attempt to pull that in. |
Switch the root and override Groovy dependency versions from the staged snapshot coordinate to the released 5.0.7 artifact. Assisted-by: opencode:openai/gpt-5.5 oracle
Use Groovy's dynamic-resolution marker to recognize taglib namespace receivers after Groovy 5 resolves getProperty(String) before the unresolved callbacks run. Assisted-by: opencode:openai/gpt-5.5 oracle
Mark the GraphQL closure helper as anchored so sub-traits can call it directly under Groovy 5.0.7, and remove the duplicated inline delegate logic. Assisted-by: opencode:openai/gpt-5.5 oracle
|
Latest update pushed in this commit set:
The PR description was also refreshed to keep only the current required Grails-side workarounds/adaptations and to move fully resolved Groovy issues into the resolved-upstream section. |
Groovy 5 marks inherited getProperty(String) lookups as dynamic before the unresolved-variable extension hook can report unknown GSP identifiers. Scan dynamic-resolution variable expressions in the GSP type-checking extension and report non-taglib names as undeclared so compile-static GSPs keep Grails' stricter model contract. Assisted-by: hephaestus:openai/gpt-5.5 oracle
|
Pushed This adds the Grails-side GSP validation pass for Groovy 5 dynamic-resolution variables, so compile-static GSPs still fail unknown bare variables the way Grails expects while preserving dynamic taglib namespace dispatch. I also updated the PR description to remove the separate list of items fully addressed on the Groovy side and keep the top-level list focused on the remaining Grails-side workarounds/adaptations. Latest local verification:
|
Pin the SiteMesh Spring artifacts in the GSP Spring Boot dependency-management example from the checked-out dependency map. This prevents a refreshed stale snapshot BOM from downgrading spring-webmvc-sitemesh to an unpublished version in CI. Assisted-by: Hephaestus:openai/gpt-5.5 oracle
|
Pushed the latest CI fix to Latest head: What changed:
Local verification:
Review gate: GREEN via Oracle session |
Merge the latest 8.0.x changes into the Groovy 5 PR branch, including the run-app PID file support and the Grails BOM test-example dependency-management updates. Assisted-by: Hephaestus:openai/gpt-5.5
|
Merged the latest What changed in this update:
Local verification run:
Review gate: GREEN from Oracle session Note: the full required aggregate command |
Assisted-by: Hephaestus:openai/gpt-5.5
This comment has been minimized.
This comment has been minimized.
…lper Groovy's experimental @Anchored trait-static annotation was voted down by the Groovy PMC and removed upstream (GROOVY-12093, replaced by @virtual), and Groovy 5.0.7 has not shipped yet, so revert the dependency coordinate from the released 5.0.7 back to 5.0.7-SNAPSHOT. @virtual only restores per-implementer override dispatch for same-trait or implementing-class calls; it does not let a child @CompileStatic trait resolve an inherited parent-trait static method (verified against 5.0.7-SNAPSHOT). Drop the @Anchored annotation/import from ExecutesClosures (keeping the plain static withDelegate as the public trait contract) and re-inline the null-safe DELEGATE_ONLY closure logic into the Arguable and ComplexTyped sub-traits, the workaround that predated @Anchored. Assisted-by: opencode:anthropic/claude-opus-4-8 oracle librarian
Update: tracking
|
| Attempt | Result |
|---|---|
plain static + ExecutesClosures.withDelegate(...) (qualified) |
STC error: Cannot find matching method java.lang.Class#withDelegate(...) |
@Virtual + ExecutesClosures.withDelegate(...) (qualified) |
same java.lang.Class#withDelegate error |
@Virtual + withDelegate(...) (unqualified, inherited) |
STC error: Cannot find matching method ...Arguable#withDelegate(...) |
This matches Groovy's own TraitStaticDispatchMatrix (rows 7/8: trait-qualified static access throws / is unsupported) and VirtualAnnotationTest (every @Virtual case is a same-trait or implementing-class call, never a child-trait→parent-trait inherited static).
The workaround. Since no @Virtual call form compiles for the cross-trait case, the GraphQL helper goes back to the approach that predated @Anchored:
ExecutesClosures.withDelegatestays a plainstaticmethod (still the trait's public contract for implementing classes - just without@Anchored/@Virtual).ArguableandComplexTypedre-inline the null-safeDELEGATE_ONLYclosure logic instead of calling the parent-trait static.
Verification (against fresh 5.0.7-SNAPSHOT, Groovy snapshot caches flushed before re-resolve):
./gradlew :grails-data-graphql-core:compileGroovy :grails-data-graphql-core:compileTestGroovy --rerun-tasks --refresh-dependencies- BUILD SUCCESSFUL./gradlew :grails-data-graphql-core:test- all specs PASSED./gradlew :grails-data-graphql-core:codeStyle- PASSED (Checkstyle + CodeNarc)git diff --check- clean
Adds Apache Groovy 5 support on top of
8.0.x, tracking the pre-release Apache Groovy5.0.7-SNAPSHOTdevelopment build.The PR is now narrowed to the Grails-side workarounds and adaptations still needed after the fixes that landed in Groovy
5.0.7-SNAPSHOT.Target stack
8.0.xlineEnd-application impact
Default Grails 8 applications that use the Grails Gradle plugin and the default
platform(grails-bom)dependency management should not need build-script changes for this Groovy 5 update.Applications that intentionally opt out of the default Grails BOM path still need to keep Groovy and Spock aligned with the Grails BOM. In particular, apps using
grails { bom = null }plusio.spring.dependency-management, or Spring Boot applications consuming Grails GSP modules outside the normal Grails platform path, must importorg.apache.grails:grails-bomand aligngroovy.version/spock.versionwith it.Application code compiled with
@CompileStatic/@GrailsCompileStaticshould also account for the Groovy 5 behavior changes already documented in the upgrade guide updates in this PR:constraints,mapping, ornamedQueriesshould qualify the field with the class name.ConfigObjectprobes that expect missing keys to be null should usecontainsKey(key) ? config.get(key) : null, sinceconfig[key]now creates an empty nestedConfigObjectfor missing keys.Grails-side workarounds and adaptations
ConstrainedProperty.DEFAULT_MESSAGESHashMapinitializer receiver.GroovyPageTypeCheckingExtensionandControllerTagLibTypeCheckingExtensiongetProperty(String)receivers as dynamic beforeunresolvedVariable/unresolvedPropertycallbacks can record Grails taglib namespace dispatch.StaticTypesMarker.DYNAMIC_RESOLUTIONto recognize taglib namespace receivers, while still requiring GSP receiver names to be configured taglib namespaces.GroovyPageTypeCheckingExtensionGSP undeclared-variable validationgetProperty(String)path.GspCompileStaticSpecchecks are enabled again.ExecutesClosures.withDelegateingrails-data-graphql@Anchoredtrait-static annotation (which let sub-traits call a parent trait'sstatichelper under@CompileStatic) was voted down and removed (GROOVY-12093). Its replacement,@groovy.transform.Virtual, only restores per-implementer override dispatch for same-trait/implementer calls; it does not make a child trait resolve an inherited parent-traitstaticunder STC (verified empirically against5.0.7-SNAPSHOT).static ExecutesClosures.withDelegateas the trait's public contract, but re-inlines the null-safeDELEGATE_ONLYclosure logic in theArguable/ComplexTypedsub-traits (the workaround that predated@Anchored), since no@Virtualcall form compiles for the cross-trait case.grails-test-examples/gsp-spring-bootSpring Dependency Management example8.0.0-SNAPSHOTBOM before the updated snapshot is published, letting Spring DM downgrade SiteMesh Spring artifacts to an unpublished version.dependencies.gradlevalues while still importinggrails-bom, keeping this regression example deterministic in CI.Local verification for the latest update
./gradlew :grails-data-graphql-core:compileGroovy :grails-data-graphql-core:compileTestGroovy --rerun-tasks --refresh-dependencies(against a freshly re-downloaded5.0.7-SNAPSHOT)./gradlew :grails-data-graphql-core:test./gradlew :grails-data-graphql-core:codeStylegit diff --checkLatest commit set
This commit set switches
dependencies.gradleback from the unreleased5.0.7coordinate to5.0.7-SNAPSHOT, and updates the GraphQL closure-delegate workaround for the upstream removal of@Anchored: it drops the@Anchoredannotation/import fromExecutesClosures(leaving a plainstatichelper as the public trait contract) and re-inlines the null-safeDELEGATE_ONLYclosure logic into theArguableandComplexTypedsub-traits, because the replacement@Virtualannotation does not let a child@CompileStatictrait resolve an inherited parent-traitstaticmethod.