Skip to content

Merge 8.0.x into 8.1.x - #16318

Closed
jamesfredley wants to merge 15 commits into
8.1.xfrom
merge/8.0.x-into-8.1.x
Closed

Merge 8.0.x into 8.1.x#16318
jamesfredley wants to merge 15 commits into
8.1.xfrom
merge/8.0.x-into-8.1.x

Conversation

@jamesfredley

Copy link
Copy Markdown
Contributor

Description

Merge current 8.0.x into 8.1.x so the 8.1 line receives the already-landed 8.0 work, including #16178 (Grails 8 compile stays indy-off via CompilePlugin).

This is a release-line merge, not a squash. Git resolved it with the ort strategy and no conflicts. projectVersion remains 8.1.0-SNAPSHOT.

Carried from 8.0.x:

Contributor Checklist

Issue and Scope

  • This PR is linked to an existing issue that has been acknowledged or approved by the project team. If no approved issue exists, please give background on why this change is necessary. Tickets are preferred for release change log history.
  • This PR addresses the complete scope of the linked issue. Partial implementations or unfinished work should not be submitted for review.
  • This PR contains a single, focused change. Unrelated changes should be submitted as separate pull requests.
  • This PR targets the correct branch for the type of change:
    • Patch release branches (e.g., 7.0.x): Bug fixes only. No new features or API changes.
    • Minor release branches (e.g., 7.1.x): New features are welcome, but breaking existing APIs must be avoided.
    • Major release branches (e.g., 8.0.x): Reserved for major changes. Breaking API changes are permitted.

Code Quality

  • I have added or updated tests that cover the changes introduced in this PR. All code contributions are expected to include appropriate test coverage.
  • I have verified that all existing tests pass by running ./gradlew build --rerun-tasks.
  • My code follows the project's code style guidelines. I have run ./gradlew codeStyle and resolved any violations. See Code Style for details.
  • This PR does not include mass reformatting, style-only changes, or large-scale refactoring unless it was explicitly approved in the linked issue. Unsolicited reformatting will not be accepted.
  • If generative AI tooling was used in preparing this contribution, a quality model was used to ensure contributions are consistent with the project's quality standards.

Licensing and Attribution

Documentation

  • If this PR introduces user-facing changes, I have included or updated the relevant documentation.
  • If this PR adds a new feature, I have updated the What's New section of the Grails Guide.
  • If this PR introduces breaking changes or changes that require user action during an upgrade, I have updated the Upgrade Notes for the corresponding version in the Grails Guide.
  • The PR description clearly explains what was changed and why.

Generative AI (Cursor Grok 4.6) was used to complete the merge.

org.gradle.jvmargs sizes the Gradle daemon only. Test forks are separate
child JVMs that take their heap from maxHeapSize in gradle/test-config.gradle,
so a job's configured heap is the daemon -Xmx plus the concurrent test forks
times the per-fork heap. Those two numbers live in different files and have
never been reasoned about together.

The concurrent fork count is not maxParallelForks. With org.gradle.parallel=true
several Test tasks run at once, so the live JVM count is bounded by Gradle's
global worker pool, which defaults to the CPU count. On the 4-CPU, ~16 GB Linux
and Windows runners that floor is 5G + 4x768m = 8G and fits. On the 3-CPU,
~7 GB macOS runner it is 5G + 3x768m = 7.25G and does not.

Cap --max-workers on the macOS leg, since that is what actually limits
concurrent test and compiler JVMs, and keep maxTestParallel alongside it so no
single task exceeds the same cap. Both are passed through a new runner_arguments
matrix key that is undefined, and therefore empty, for every other entry.

The daemon stays at 5 GB: groovydoc is what needs it, and shrinking it would
trade a memory problem for a slower build. Document the arithmetic next to
org.gradle.jvmargs as a simplified configured-heap floor, explicitly excluding
metaspace, native memory and the forked compiler workers that CompilePlugin
gives their own -Xmx2G, so it is not mistaken for a true peak.

This changes concurrency only. No test is added, removed, skipped or weakened.

Assisted-by: claude-code:claude-opus-5
Groovy 5 defaults indy on. Only modules that apply the Grails Gradle
plugin inherited grails.indy=false, so published framework artifacts
were mixed. Centralize indy=false in CompilePlugin and apply
gradle/groovy-indy.gradle from the grails-core, grails-gradle, and
grails-forge builds. CI can still opt in with -PgrailsIndy=true.

See #15293

Assisted-by: Sisyphus:grok-4.6
With indy off, log.debug inside GormStaticApi.count()'s session
callback was dispatched as Domain.debug(...). Capture the @slf4j
logger in a local first. Also trim -PgrailsIndy the same way as
CompilePlugin and apply the shared groovy-indy script from
build-logic.

Assisted-by: Sisyphus:grok-4.6
Class.newInstance(Map) is not selected under @CompileStatic when
invokedynamic is disabled. Use InvokerHelper.invokeConstructorOf with
an explicit Object[] so nested Map-constructor types still bind.
Also avoid `null as boolean` in the Map-constructor test fixture,
which Groovy 5 throws on without indy after unbindable properties
are filtered from constructor arguments.

Assisted-by: Sisyphus:grok-4.6
Honor review: drop groovy-indy.gradle and subprojects applies. Unpublished build-logic uses Gradle's Groovy default.
Hibernate 7 publishes jboss-logging as a runtime-only transitive.
Groovydoc Class.forName's referenced types, so
:grails-data-hibernate7-dbmigration-core:groovydoc failed in CI
(Forge/e2e publish) with NoClassDefFoundError: org/jboss/logging/Logger.
build(ci): bound concurrent JVMs on the macOS runner
build: disable Groovy invokedynamic for the Grails 8 compile
Copilot AI lite review requested due to automatic review settings September 4, 2026 23:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes appear consistent with the stated merge goals, include targeted regression tests for the key indy-off behaviors, and do not introduce confirmed correctness or configuration hazards in the reviewed hunks.

Pull request overview

This PR merges the current 8.0.x line into 8.1.x, bringing forward build defaults and runtime/test fixes needed for Groovy invokedynamic disabled by default (via CompilePlugin), along with CI and Groovydoc reliability improvements.

Changes:

  • Centralize Groovy invokedynamic (indy) default to false in CompilePlugin, while keeping -PgrailsIndy=true as an opt-in (including whitespace-tolerant parsing).
  • Fix databinding instantiation for nested types that only expose Map constructors under @CompileStatic + indy-off, and adjust tests accordingly.
  • Improve Groovydoc robustness by extending its classpath with runtime dependencies, and constrain macOS CI concurrency to reduce memory pressure.
File summaries
File Description
grails-web-databinding/src/main/groovy/grails/web/databinding/GrailsWebDataBinder.groovy Uses shared Map-constructor instantiation helper for nested binding under indy-off.
grails-test-suite-persistence/src/test/groovy/grails/web/databinding/GrailsWebDataBinderSpec.groovy Adjusts Map-constructor test fixture to avoid null as boolean under indy-off.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormStaticApi.groovy Prevents logger calls in count() from being dispatched through methodMissing by capturing the logger before entering callbacks.
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormStaticApiSpec.groovy Adds regression coverage ensuring count() runs without triggering the methodMissing/logger dispatch failure.
grails-databinding-core/src/main/groovy/grails/databinding/SimpleDataBinder.groovy Introduces newInstanceFromMapArguments(...) using InvokerHelper to reliably invoke Map ctors under @CompileStatic with indy disabled.
gradle/grails-extension-gradle-config.gradle Makes grailsIndy parsing whitespace-tolerant and consistent with the build-logic property lookup behavior.
gradle.properties Documents CI/local heap budgeting and why macOS caps workers rather than shrinking the daemon heap.
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/CompilePlugin.groovy Sets the default GroovyCompile indy flag via grailsIndy property (default false) for published artifacts.
build-logic/plugins/src/test/groovy/org/apache/grails/buildsrc/CompilePluginSpec.groovy Adds TestKit coverage for indy default/override and whitespace trimming.
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GroovydocEnhancerPlugin.groovy Extends Groovydoc task classpath with runtimeClasspath to avoid NoClassDefFoundError from runtime-only transitives.
build-logic/plugins/src/test/groovy/org/apache/grails/buildsrc/GroovydocEnhancerPluginSpec.groovy Adds coverage ensuring Groovydoc sees runtime-only jars on its classpath.
.github/workflows/gradle.yml Caps macOS --max-workers and test parallelism via matrix runner_arguments.
Review details
  • Files reviewed: 12/12 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.7075%. Comparing base (2545945) to head (42a30d7).

Files with missing lines Patch % Lines
.../grails/web/databinding/GrailsWebDataBinder.groovy 0.0000% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##                8.1.x     #16318        +/-   ##
==================================================
- Coverage     55.8236%   55.7075%   -0.1161%     
- Complexity      21437      21441         +4     
==================================================
  Files            2146       2146                
  Lines          102616     102663        +47     
  Branches        18160      18197        +37     
==================================================
- Hits            57284      57191        -93     
  Misses          37382      37382                
- Partials         7950       8090       +140     
Files with missing lines Coverage Δ
.../groovy/grails/databinding/SimpleDataBinder.groovy 74.3210% <100.0000%> (-0.1840%) ⬇️
...ovy/org/grails/datastore/gorm/GormStaticApi.groovy 76.1290% <100.0000%> (-0.2464%) ⬇️
.../grails/web/databinding/GrailsWebDataBinder.groovy 30.0341% <0.0000%> (-0.0513%) ⬇️

... and 64 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codeconsole

Copy link
Copy Markdown
Contributor

You are creating new merges from 8.0.x when 8.0.x is currently in a broken state from a merge from. 7.x

Shouldn't we be doing merges sequentially? Why create 8.1.x into 9.0.x before 8.0.x has been merged into 8.1.x?

@testlens-app

testlens-app Bot commented Sep 5, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 42a30d7
▶️ Tests: 74531 executed
⚪️ Checks: 85/85 completed


Learn more about TestLens at testlens.app/docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants