fix: resolve the Spring DM example against the in-commit BOM - #16073
fix: resolve the Spring DM example against the in-commit BOM#16073jamesfredley wants to merge 4 commits into
Conversation
The Spring Dependency Management example opts out of the native platform(grails-bom) injection and imports grails-bom as a Maven BOM instead. io.spring.dependency-management resolves that import as an artifact-only @pom in its own detached configuration, which never sees the project substitution in gradle/functional-test-config.gradle, so it could only ever be satisfied from a repository. That had two consequences. The example silently validated against the last BOM published to the Apache snapshot repository rather than the one in the commit under test, so a change to dependencies.gradle went unverified there and the effective BOM changed whenever CI published. And on a version that had never been published - a new release branch, immediately after the version bump - the import produced no managed versions at all rather than failing, so every managed dependency resolved with an empty version and the build died with "Could not find <group>:<artifact>:", before CI could publish the snapshot that would have fixed it. Generate the BOM poms this build produces and serve them from a local repository instead: - Stage grails-base-bom, grails-bom, grails-hibernate5-bom and grails-hibernate7-bom into .gradle/local-boms during root configuration. Spring DM resolves its detached import while the task graph is still being computed, so no task dependency can put the poms in place in time. The location is outside build/ so a combined `gradlew clean <task>` cannot delete them between configuration and resolution. - Serve them through an exclusiveContent repository scoped to this build, so the coordinates resolve locally or fail loudly rather than falling back to the remote and quietly reintroducing the stale-BOM behaviour. - Evaluate grails-base-bom last. It classifies sibling projects by whether they already carry java-platform, so evaluating it first made it adopt the other BOMs as managed dependencies and bake that into the published pom. - Stop BomPropertyOverridesPlugin resolving project platforms as external modules. It requested grails.core.ROOT:grails-hibernate5-bom:unspecified, which cannot resolve, leaving those versions unmanaged. The temporary -PbomSnapshotNotPublished guard is no longer needed and the example is included unconditionally again, including during reproducible release builds. Assisted-by: claude-code:claude-opus-5
There was a problem hiding this comment.
Pull request overview
This PR fixes the grails-test-examples/spring-dependency-management build so it validates the BOM generated by the current checkout (not the last published BOM) and no longer breaks when projectVersion is bumped to an unpublished version by serving in-commit BOM POMs from a local, exclusive Maven repository.
Changes:
- Always include the Spring Dependency Management example and make it consume the in-commit BOM artifacts.
- Generate/stage BOM POMs into
.gradle/local-bomsduring root configuration and expose them via anexclusiveContentsettings repository. - Fix
BomPropertyOverridesPluginplatform detection to ignore project platform dependencies (and add coverage for that behavior).
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| settings.gradle | Always includes the Spring DM example now that it can resolve BOMs locally. |
| build.gradle | Generates BOM POMs at configuration time and copies them into .gradle/local-boms for the example to consume. |
| build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsRepoSettingsPlugin.groovy | Adds an exclusive, file-based Maven repo pointing at .gradle/local-boms for BOM coordinates. |
| grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/bom/BomPropertyOverridesPlugin.groovy | Skips ProjectDependency platforms when auto-detecting declared BOMs. |
| grails-gradle/plugins/src/test/groovy/org/grails/gradle/plugin/bom/BomPropertyOverridesPluginSpec.groovy | Adds a test ensuring project platform dependencies are ignored. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The comment correctly identifies that the implementation uses a file-based repository located at build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsRepoSettingsPlugin.groovy |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #16073 +/- ##
================================================
+ Coverage 0 51.8779% +51.8779%
- Complexity 0 18114 +18114
================================================
Files 0 2046 +2046
Lines 0 96276 +96276
Branches 0 16728 +16728
================================================
+ Hits 0 49946 +49946
- Misses 0 38957 +38957
- Partials 0 7373 +7373
🚀 New features to boost your workflow:
|
… mavenLocal() Assisted-by: claude-code:claude-opus-5
The example imported org.apache.grails:grails-bom through io.spring.dependency-management, which resolves BOM imports in its own detached configuration. That configuration never sees the project substitution in gradle/functional-test-config.gradle, so the import could only ever be satisfied from a repository - which meant the example validated whichever BOM was last published rather than the one in the commit, and failed outright on a version that had never been published, taking CI down on a newly created release branch before it could publish anything. Serving the in-commit BOM from a local repository was tried and abandoned: generating the poms early enough for Spring DM realizes the BOM projects before gradle/cli-companion-bom-constraints.gradle has discovered cliArtifactId, which silently drops every CLI companion constraint from the generated and published BOM, and generating them late enough for a complete BOM is already too late for the import to see them. Drop the BOM artifact import instead and manage the example's versions from dependencies.gradle, which is the single source of truth and which this example already read for its logback and jackson overrides. The example still sets bom = null and still applies io.spring.dependency-management, so it continues to cover a migrated Grails 7 application whose versions are managed by Spring DM rather than by the platform. Spring Boot's own BOM is still imported automatically and silently supplied older Groovy, commons-codec and log4j versions wherever an explicit entry was missing, so its version properties are overridden from dependencies.gradle as well. verifyDependencyManagementVersions asserts the resolved versions on the compile and integration test runtime classpaths and fails if any of them drift, since that divergence is invisible to a successful build. Assisted-by: claude-code:claude-opus-5
Updated - the regression is gone and this is ready for review
What the branch now does. The example stops importing A second defect was found and fixed along the way. Spring Boot's BOM is still imported automatically, and wherever an explicit entry was missing it silently supplied an older version:
Fixed by overriding the corresponding Spring Boot version properties from
The assertion is proven in both directions: it passes as committed, and with Verification
Known follow-ups
Trade-off for reviewers to confirmThe example no longer exercises consuming a real Maven BOM artifact through Spring DM. That path never validated the commit under test - it always resolved the last published BOM - and Grails 8 applications use the platform BOM. Both automated reviewers flagged this as the deliberate cost of the change, so it should be an explicit decision rather than a silent one. The |
verifyDependencyManagementVersions failed the SiteMesh 2 lane, which sets SITEMESH2_TESTING_ENABLED and resolves no Jackson at all, because the guard treated a coordinate missing from every checked configuration as evidence that the check had stopped working. That guard was written for a real failure - an earlier revision keyed its lookup map with a GString, so every lookup missed and the task asserted nothing - but it cannot distinguish that from a variant legitimately not using a dependency. Fail only when no expected coordinate resolves anywhere, which is what the broken-lookup case actually looks like. Per-coordinate version mismatches still fail as before. Assisted-by: claude-code:claude-opus-5
✅ All tests passed ✅🏷️ Commit: 6efd359 Learn more about TestLens at testlens.app. |
|
FYI: Previously, we solved these type of failures by using From what I can tell, a version of this was already committed to 8.x. The purpose of the example being changed is to use the maven import. We shouldn't be circumventing that since that's the default behavior of the plugin. We recently started publishing core to a local directory (forge, end to end tests). We should move it there. #16077 does this and I'd like to close this in favor of that PR. |
|
Replaced by #16077 |
Summary
grails-test-examples/spring-dependency-managementhas never actually tested the commit it runs on, and it takes the whole build down wheneverprojectVersionis bumped to a version that has not been published yet.This drops the
grails-bomartifact import from the example and manages its versions fromdependencies.gradleinstead - the same single source of truth the BOM itself is built from - so the example is validated against the tree under test and no longer depends on anything having been published.Reviewers: the interesting parts are the example's
build.gradleandBomPropertyOverridesPlugin.groovy. The rest follows from those.Background
The example deliberately opts out of the native platform injection and imported the BOM the old way, to keep coverage for applications migrated from Grails 7:
grails { bom = null } apply plugin: 'io.spring.dependency-management' dependencyManagement { imports { mavenBom "org.apache.grails:grails-bom:${projectVersion}" } }io.spring.dependency-managementresolves that import as an artifact-only@pomrequest inside its own detached configuration. Detached configurations never see thedependencySubstitutionrules ingradle/functional-test-config.gradle, which is what mapsorg.apache.grails:*onto local projects for every other example. So this import could only ever be satisfied from a real repository, and the only repository that had the artifact was the remote Apache snapshot repo.Consequence 1 - it validated the wrong BOM, silently
Because the import came from the remote, the example was managed by whatever was published last, not by the tree under test. So any PR touching
dependencies.gradlewent unverified by this example, and on8.0.xthe BOM it tested against changed underneath it every time CI published.Consequence 2 - it broke every new release branch
A failed BOM import is not an error to Spring DM. It yields an empty managed-version map, so every managed dependency then resolves with an empty version:
On a freshly created release branch the bumped version has never been published, so this fired immediately and took roughly 30 CI jobs down.
publishis gated behindbuild, so CI could not publish the snapshot that would have fixed it - the branch could not bootstrap itself. This happened on8.1.xand was worked around with the temporary-PbomSnapshotNotPublishedflag added in 57883c4, which this PR removes.Approach that was tried and abandoned
Earlier revisions of this PR generated the in-commit BOM poms into
.gradle/local-bomsduring root configuration and served them through anexclusiveContentrepository, keeping themavenBomimport intact. That is visible in the commit history and is not what this PR does now.It was dropped because the timing has no valid window. Spring DM resolves its detached import while Gradle is still computing the task graph, so the poms have to be generated during root configuration - but generating them that early realizes the BOM projects before
gradle/cli-companion-bom-constraints.gradlehas discoveredcliArtifactId, which silently drops every CLI companion constraint from the generated and published BOM. Generating them late enough to produce a complete BOM is already too late for the import to see them. Fixing a silent-wrong-BOM bug by introducing a silent-incomplete-BOM bug is not a fix.What changed
1. The example manages its own versions from
dependencies.gradle(grails-test-examples/spring-dependency-management/build.gradle)The
mavenBomimport is gone. The example already readdependencies.gradlefor its logback and Jackson 3 CVE overrides; it now reads it for everything it needs, via adependencyManagement { dependencies { ... } }block plus Spring Boot version-property overrides.It still sets
bom = nulland still appliesio.spring.dependency-management, so it continues to cover a migrated Grails 7 application whose versions are managed by Spring DM rather than by the platform - which is the point of the example.Spring Boot's own BOM is still imported automatically by the plugin, and it was silently supplying older Groovy, commons-codec and log4j versions wherever the Grails BOM had no explicit entry. Those version properties (
groovy.version,commons-codec.version,log4j2.version, alongside the existinglogback.versionandjackson-bom.version) are now overridden fromdependencies.gradletoo.2. A guard so this cannot silently rot (
verifyDependencyManagementVersions)Version drift here is invisible to a successful build - that is exactly how the original bug survived. The new task asserts the resolved versions on
compileClasspathandintegrationTestRuntimeClasspathagainstdependencies.gradle, plus a blanket check that every resolvedorg.apache.groovy:*artifact matchesgroovy.version. It is wired intocheck.Two deliberate design points, both learned from failures during this PR:
GString, so every lookup missed and the task passed while asserting nothing.3. Fix project-platform detection (
BomPropertyOverridesPlugin.groovy)The plugin was requesting
Note for reviewers: this is a shared Gradle plugin used by real applications, so it is the highest-risk file here and deserves the closest look.
4. Drop the temporary guard (
settings.gradle)-PbomSnapshotNotPublishedis gone and the example is included unconditionally, including during reproducible release builds - the originalisReproducibleBuildexclusion existed for this same "BOM not published yet" reason and is no longer needed.Verification
-PprojectVersion=9.9.9-SNAPSHOT, a version that has never existed8.0.0-SNAPSHOTwith no remote BOM available:grails-test-examples-spring-dependency-management:check(runs the new verification task)SITEMESH2_TESTING_ENABLED):grails-gradle-plugins:test --tests "*BomPropertyOverridesPluginSpec*"Known follow-ups
Deliberately out of scope, worth tracking separately:
dependencies.gradlewill not automatically reach this example.verifyDependencyManagementVersionscatches drift in what is listed, not omissions. Reconnecting the example to the real BOM without the timing problem above is the proper long-term fix.BomPropertyOverridesPluginnow skips allProjectDependencyplatforms, which is broader than its documented "auto-detects declared platforms" behaviour. Consumers relying on project platforms must declare them explicitly withbom(...). This should either be narrowed or documented, with functional coverage.Merge-up
Targets
8.0.x. Once merged,8.1.xand9.0.xneedbomSnapshotNotPublished=truedeleted fromgradle.properties- it becomes a no-op immediately and a dead property after that.