jamesfredley commented on PR #15558: URL: https://github.com/apache/grails-core/pull/15558#issuecomment-4322752325
## Final wrap-up: Groovy 5 / Groovy 6 workaround audit complete This is the final state of the workaround audit after end-to-end integration testing against Apache Groovy 6.0.0-SNAPSHOT master HEAD (build 508-516; verified that builds 509-516 add only Javadoc + 1 build-infra refactor on top of build 508 - so no functional delta vs. master HEAD). ### Bottom line **5 Groovy 5 workarounds removed in this audit. 7 confirmed required. 2 will be removable when 2 OPEN upstream PRs merge. 1 is Spring 7 specific. 3 untestable locally (Forge canary red for unrelated reasons).** ### Methodology recap 1. Read every `// Groovy 5` and `// GROOVY-` comment in the working tree 2. For each, attempted removal and ran the affected module's test suite on Groovy 6.0.0-SNAPSHOT build 508 3. Verified MongoDB integration tests with live `mongo:7.0` TestContainer (478 tests passed with `BsonPersistentEntityCodec.resolvePropertyType()` walker removed) 4. Verified Geb indy=false integration tests with `selenium/standalone-chrome:latest` TestContainer (reproduced `VerifyError: get long/double overflows locals` on `ContainerSupport$Trait$Helper.createFileInputSource @0: dload_3` when `@CompileDynamic` reverted - the workaround stays) 5. **Cloned `apache/groovy` master, fetched OPEN PRs #2492 and #2493, built each locally with `./gradlew publishToMavenLocal`, replaced cached SNAPSHOT JARs, reverted the corresponding Grails workaround, and re-ran the affected tests to confirm both upstream PRs unblock our workarounds** ### Verified upstream PRs that unblock our remaining workarounds (NOT yet merged to master) | Upstream | Local verification | Grails workaround it unblocks | |---|---|---| | **OPEN PR [apache/groovy#2492](https://github.com/apache/groovy/pull/2492)** ([GROOVY-11966](https://issues.apache.org/jira/browse/GROOVY-11966), Paul King) - synchronises `NodeMetaDataHandler.getNodeMetaData` map access | Built locally, replaced cache JAR (NodeMetaDataHandler `monitorenter` count went from 0 to 7), reverted both parallelism guards, ran 8 parallel `:grails-test-examples-*:compileGroovyPages` - `BUILD SUCCESSFUL`, 0 ListHashMap errors. Race only reliably reproduces in CI (timing-dependent) so we couldn't reliably reproduce the negative case locally, but the synchronisation is a textbook fix for the documented `ArrayIndexOutOfBoundsException` race | `grails-gsp/core/.../GroovyPageCompiler.groovy` parallelism guard + `grails-views-core/.../AbstractGroovyTemplateCompiler.groovy` parallelism guard | | **OPEN PR [apache/groovy#2493](https://github.com/apache/groovy/pull/2493)** ([GROOVY-11967](https://issues.apache.org/jira/browse/GROOVY-11967), Paul King) - adds `CHECKCAST` to indy-mode `ListExpressionTransformer` to fix the `VerifyError` on the synthesised lower-arity bridge constructor | Built locally, replaced cache JAR (verified `InvokeDynamicWriter` reference now in `ListExpressionTransformer$NewListExpression.class`), reverted `DefaultConstraintFactory` + `MappingContextAwareConstraintFactory` back to single-constructor form with `targetTypes = [Object] as List<Class>` default value, ran `:grails-validation:test :grails-datamapping-validation:test` under both indy=false and indy=true - `BUILD SUCCESSFUL` | `grails-datamapping-validation/.../DefaultConstraintFactory.groovy` two-ctor split + `grails-datamapping-core/.../MappingContextAwareConstraintFactory.groovy` two-ctor split | ### Verified upstream JIRAs already in build 508+ master HEAD | JIRA | Status | Fix commit | Already-removed Grails workaround | |---|---|---|---| | [GROOVY-11907](https://issues.apache.org/jira/browse/GROOVY-11907) "trait static field helper generates invalid bytecode" | Resolved (5.0.6) | `19f38997a` (2026-04-08) | `HibernateEntity` static SQL methods (commit `8af5d1dc4c`), `JspTagImpl` `@CompileDynamic` (commit `2bb0930a5d`), `ClassPropertyFetcherTests` generic trait (commit `a71c8b5ebb`), `GormEntityTransformation` AST shim path (now unconditional, commit `8e9cdbc50f`), `MongoCodecSession` increment, scaffolding GROOVY-11907 trait statics (commit `a290b37156`). **Note: indy=false static-setter helper path is NOT covered** - reproduced today on `ContainerSupport`, needs an upstream follow-up filed | | [GROOVY-11911](https://issues.apache.org/jira/browse/GROOVY-11911) "Restore Groovy 5's MOP-aware call dispatch for Java Closure subclasses overriding call(Object) without doCall" | Resolved (master) | `ac71deb` (2026-04-26 07:19 UTC, in build 508) | **`ControllerActionTransformer` Closure dispatch workaround (REMOVED in this audit)** - reverted Java for-loop back to `DefaultGroovyMethods.count(Iterable, Closure)` form, all 133 `:grails-controllers:test :grails-rest-transforms:test` tasks green | | [GROOVY-11512](https://issues.apache.org/jira/browse/GROOVY-11512) "Inconsistent isAttribute & getAttribute behavior in Groovy 4 with traits" | Resolved (4.0.28 / 5.0.0-beta-2 / 6.0.0-alpha) | `88c63360` (2024-11-01) | `TraitPropertyAccessStrategy` (inherited from base PR #15557, not retested in this canary) | | [GROOVY-11829](https://issues.apache.org/jira/browse/GROOVY-11829) "Properties located from a set(key, value) always use the same method even when the value type is better matched by another" | Resolved (6.0.0-alpha-1) | `7bc29825bc` (2026-01-01) | This addresses `set(...)` not `get(...)` - it does NOT address our `MetaClassImpl.isGenericGetMethod` instance-dispatch hijack, which is why our `GormEntityTransformation` instance `get(String)` shim is still required | | [GROOVY-11522](https://issues.apache.org/jira/browse/GROOVY-11522) "Possible Null Pointer Dereference in VariableScopeVisitor" | Resolved (4.0.28 / 5.0.0-beta-2 / 3.0.26) | `f5666584e1` (2026-02-24) | This addresses a different `findClassMember` NPE - it does NOT address our 4 `visitConstructorOrMethod` NPE catches, which still reproduce on master HEAD when removed. Our NPE class needs a separate upstream filing | ### Issues that NEED to be filed upstream (no matching JIRA / PR found) | Issue | Caused workaround | |---|---| | `MetaClassImpl` static-trait `get(String)` hijacking instance dispatch (distinct from GROOVY-11829 which addresses `set(key, value)`) | `GormEntityTransformation` instance `get(String)` shim | | Our specific `VariableScopeVisitor.visitConstructorOrMethod` NPE class (4 catch sites + 2 null-VariableScope ClosureWriter sites; one umbrella bug family per Oracle review) | `GrailsASTUtils`, `AstUtils`, `AbstractMethodDecoratingTransformation`, `ResourceTransform`, `LoggingTransformer` | | `@Delegate` field on trait silently returns null on Groovy 5/6 lowering (separate from GROOVY-11512) | `GrailsApplicationCommand` trait → abstract class | | `TraitReceiverTransformer` static override loss when calling `this.method()` from a trait static method | `Validateable.resolveDefaultNullable` reflection lookup | | `ConfigObject` infinite recursion under `Map` iteration (Groovy 4 → 5 behavior change carried into 6, no JIRA filed since 2014) | `NavigableMap.convertConfigObjectToMap()` shallow + lazy conversion | | Interface `$getCallSiteArray()` `IncompatibleClassChangeError` under indy=false | `IContainerGebConfiguration` interface → trait | | GROOVY-11907 follow-up: indy=false static-setter trait helper bytecode (verified today: `dload_3` overflow on 2-local frame) | `ContainerSupport` `@CompileDynamic` | ### Forge integration tests (3 workarounds remaining there) `GrailsApplicationCommand` (trait → abstract class), `TemplateRendererImpl` (explicit type checks), and `GenerateControllerCommand` (explicit 4-arg render calls) all compile cleanly when reverted. Their failure mode is silent runtime `@Delegate` returning null, only catchable by Forge `ScaffoldingSpec.test generate-controller command` integration test. **`Build Grails Forge` is currently red on this branch's CI for an unrelated `compileTestGroovy` failure** (was already red on the previous canary CI run before any of these fixes), so we cannot get a clean signal. These workarounds stay until Forge canary goes green. ### Net delta - Working tree: 14 files changed, **5 source workarounds removed** (-128 / +51 lines), 9 inline comment updates from "Groovy 5" → "Groovy 5/6" with reproducer details on the workarounds confirmed still required. - All 530 modules compile clean: `./gradlew classes` -> `BUILD SUCCESSFUL in 2m 10s`. - No commit yet - waiting for review. Assisted-by: claude-code:claude-opus-4-7 -- 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]
