jamesfredley commented on PR #15558: URL: https://github.com/apache/grails-core/pull/15558#issuecomment-4322495123
## Groovy 5 → Groovy 6 workaround audit (round 2 - all carry-forward workarounds tested) This is the second round of testing, covering the items previously deferred. Methodology unchanged from round 1: revert workaround → run affected module's full test suite on Groovy 6.0.0-SNAPSHOT build 508 (`groovy-6.0.0-SNAPSHOT.jar` from `repository.apache.org/snapshots`, fetched 2026-04-26 13:47 UTC, JDK 21, Gradle 9.4.1) → keep removed if green, restore with citation if red. Build 514 is now the latest published snapshot (2026-04-26 16:17 UTC) but builds 509-514 only added Javadoc commits - no functional changes. So build 508 reflects current `apache/groovy` master HEAD for our purposes. ### Round 2 — Workarounds REMOVED (confirmed no longer needed on Groovy 6) | File | Workaround removed | Verification | Upstream fix | |---|---|---|---| | `grails-core/.../core/cfg/GroovyConfigPropertySourceLoader.groovy` | Recursive `toRegularMap(ConfigObject)` deep-conversion that walked the entire ConfigObject tree before passing to `propertySource.merge(...)`. Also removed the unused `@CompileDynamic` import. | `:grails-core:test --rerun-tasks` (45/45 tasks executed, all green) - including `GroovyConfigPropertySourceLoaderSpec.test loading multiple configuration files` which exercises the merge path | The NavigableMap-side shallow + lazy conversion handles ConfigObject correctly; the deep pre-conversion was redundant once that landed. Not a Groovy 6 fix per se; a Grails-side simplification. | | `grails-data-hibernate5/core/.../compiler/HibernateEntityTransformation.groovy` | `if (InnerClassNode.isAssignableFrom(classNode.getClass()) || classNode.isEnum())` workaround for the Groovy 5 `@CompileStatic` `instanceof`-emits-`checkcast` bytecode bug. Reverted to plain `if (classNode instanceof InnerClassNode || classNode.isEnum())` | `:grails-data-hibernate5-core:test --rerun-tasks` (79/79 tasks executed, 3m 34s, all green) | `instanceof` bytecode generation fixed in Groovy 6 (specific JIRA not located but verified by behaviour) | | `grails-controllers/.../web/ControllerActionTransformer.java` | Plain Java for-loop replacing `DefaultGroovyMethods.count(Iterable, Closure)` + anonymous Closure subclass overriding `call(Object)`. Reverted to original Closure form, restored `DefaultGroovyMethods` import. | `:grails-controllers:test :grails-rest-transforms:test --rerun-tasks` (133/133 tasks, 1m 17s, all green) - critically including `:grails-rest-transforms:compileGroovy` which was the original failure site | **[GROOVY-11911](https://issues.apache.org/jira/browse/GROOVY-11911)** "Deprecate callsite versions of BooleanClosureWrapper (restore Groovy 5's MOP-aware call dispatch for Java Closure subclasses that override call(Object) without a doCall)" - commit `ac71deb` 2026-04-26 07:19 UTC. **In build 508.** | ### Round 2 — Workarounds NOT TESTABLE in unit tests (compile passes when reverted; runtime detection requires integration test environment) | File | Workaround | Why kept defensively | |---|---|---| | `grails-core/.../GrailsApplicationCommand.groovy` (and 9 subclasses) | `abstract class` instead of `trait`, with `@Delegate` fields working via standard class field access. Reverting requires changing 1 trait + 9 subclasses (`extends` ↔ `implements`). | Compile passes for both forms. Runtime failure mode is `@Delegate` fields silently returning null - only the Forge `ScaffoldingSpec.test generate-controller command` integration test catches it, and that suite is already red on this branch for an unrelated `compileTestGroovy` issue. Not retesting until Forge canary is fixed. | | `grails-core/.../template/TemplateRendererImpl.groovy` | Explicit `Resource`/`File` type checks in `render(Map)` bypassing `@Delegate` named-arg bridge | Compile passes when simplified back to `template(...)` / `file(...)` calls. Same forge-only runtime detection. | | `grails-scaffolding/.../GenerateControllerCommand.groovy` | Explicit 4-arg `templateRenderer.render(Resource, File, Model, boolean)` calls via a `generateFile(...)` helper | Compile passes when reverted to named-arg `render(template:..., destination:..., model:..., overwrite:...)`. Same forge-only runtime detection. | | `grails-data-mongodb/bson/.../BsonPersistentEntityCodec.groovy` | `resolvePropertyType()` walker for synthetic intermediate superclasses on anonymous-inner-class properties from `MappingFactory` | BSON unit tests pass with the simple `propertyClass.superclass` form, but the synthetic-class scenario only surfaces during MongoDB persistence with `@CompileStatic` anonymous inner classes - needs live MongoDB integration test | | `grails-datastore-core/.../config/ConfigurationBuilder.groovy` | (a) `Map.isAssignableFrom()` exclusion removed for Grails-package Map subtypes (`HibernateSettings extends LinkedHashMap`). (b) Abstract/interface exclusion. (c) `handleConverterNotFoundException` Spring 7 fallback. | **Spring 7 / Spring Boot 4 compatibility, NOT Groovy-version-conditional.** Cannot be removed by upgrading Groovy alone. The Map subtype exclusion logic and `ConverterNotFoundException` fallback are about Spring's `MapToMapConverter` and `Configurable` builder strategy detection, both of which behave the same on Groovy 5 and Groovy 6. | | `grails-geb/.../testFixtures/grails/plugin/geb/ContainerGebConfiguration.groovy` | `IContainerGebConfiguration` as a trait instead of an `interface` with default methods | Compile passes for both forms. Runtime failure mode is `IncompatibleClassChangeError` at class-load time when downstream consumer is compiled with `-PgrailsIndy=false`. Already retested twice in commits `b8ee60d460` and `b0c7f347e8` and re-added because of the indy=false matrix failure. | | `grails-geb/.../testFixtures/grails/plugin/geb/support/ContainerSupport.groovy` | `@CompileDynamic` instead of `@CompileStatic` on a trait with static fields | GROOVY-11907 fix is incomplete for indy=false consumers - `VerifyError: get long/double overflows locals` at `ContainerSupport$Trait$Helper.setContainer`. Already retested in commit `0804a4fc73`. | | `grails-geb/.../testFixtures/grails/plugin/geb/WebDriverContainerHolder.groovy` | `getOverriddenProperties()` static-type-check helper | Static-type inference helper - low risk to keep. | ### Net round 2 result - **3 more workarounds removed** (`GroovyConfigPropertySourceLoader`, `HibernateEntityTransformation` `instanceof`, `ControllerActionTransformer` closure) - **1 of those** (`ControllerActionTransformer`) traceable to a confirmed upstream fix: [GROOVY-11911](https://issues.apache.org/jira/browse/GROOVY-11911), already in build 508 - **8 workarounds confirmed kept defensively** for either Spring 7 compatibility or runtime-only failure modes (forge / indy=false / MongoDB integration) that unit tests don't catch ### Combined with round 1 Total **4 Groovy 5 workarounds removed** in this PR's audit: 1. `AbstractConstraint.java` message bundle fallback (round 1) 2. `GroovyConfigPropertySourceLoader.groovy` recursive normalization (round 2) 3. `HibernateEntityTransformation.groovy` `instanceof` workaround (round 2) 4. `ControllerActionTransformer.java` Closure dispatch workaround (round 2 - cites GROOVY-11911) ### Apache Groovy upstream sanity check A maintainer indicated the listed regressions "should already have fixes/PRs in place" for Groovy 6. I searched `apache/groovy` master commits and JIRA directly: | Issue | Upstream status | |---|---| | Closure dispatch (`count(Iterable, Closure)` MOP `doCall`) | **Filed and fixed**: [GROOVY-11911](https://issues.apache.org/jira/browse/GROOVY-11911), commit `ac71deb`, in build 508. Workaround removed. | | GROOVY-11907 trait static field bytecode | **Fixed in 5.0.6**, merge `f7305b313`, in build 508. Most workarounds removed earlier. **`ContainerSupport` indy=false case still hits a follow-up bug** - see commit `0804a4fc73`. | | `MetaClassImpl.isGenericGetMethod` regression | Not filed. No matching commits in master. | | `VariableScopeVisitor` NPE class | Not filed. Recent `VariableScopeVisitor` commits in master (`f5666584`, `23d9322e`, both 2026-02-24) tweak `findClassMember`, not the `visitConstructorOrMethod` NPE we hit. | | `TraitReceiverTransformer` static override loss | Not filed. Most recent `TraitReceiverTransformer` commit is `19f38997a` for GROOVY-11907 (2026-04-08). | | `ListHashMap` thread-safety regression | Not filed. No 2025-2026 `ListHashMap` commits. | | `@CompileStatic` default-valued `List<Class>` VerifyError | Not filed. | | `ConfigObject` infinite recursion under `Map` iteration | Not filed in 2025-2026. ConfigObject commits are all 2012-2014 (cosmetic). | | Interface `$getCallSiteArray()` ICCE under indy=false | Not filed. | (Initial automated lookup via the `librarian` agent fabricated JIRA numbers GROOVY-11966 through GROOVY-11971 with bogus commit SHAs that turned out to be unrelated Javadoc-only commits when verified via `gh api repos/apache/groovy/commits/<sha>`. The table above reflects real findings only.) If a maintainer can confirm the actual JIRA / PR numbers for the remaining 7 issues, I will retest each one and either (a) remove the workaround if the fix is in build 508, or (b) build apache/groovy locally and retest if the fix is post-snapshot. 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]
