jamesfredley commented on PR #15467:
URL: https://github.com/apache/grails-core/pull/15467#issuecomment-4513887163
## Final pass (commits `48de2b8` + `8829323`) - ready for merge
Three concrete changes that close out the last of the review feedback and
remove every remaining Spring Dependency Management coupling from the entire
repo.
### 1. `grails { autoApplyBom }` opt-out (thread `AJjff`)
`GrailsExtension` gains an `autoApplyBom` `Property<Boolean>`. Default is
`true`, which preserves the implicit Grails 7 behaviour (Spring DM always
applied the BOM). Setting `grails { autoApplyBom = false }` in a project's
`build.gradle` now suppresses both the automatic `platform(grails-bom)`
injection AND the application of the `bom-property-overrides` plugin - one
clean knob for the entire BOM automation.
`applyGrailsBom()` is wrapped in `afterEvaluate` so the flag is read after
the user's `build.gradle` has finished evaluating. New `AutoApplyBomSpec`
(TestKit) verifies the opt-out path end-to-end.
Documented in upgrade-guide section 14 with a `build.gradle` example.
### 2. Services-based BOM resolution (thread `AJhsl`)
`BomManagedVersions.resolve()` and
`BomPropertyOverridesPlugin.applyOverrides()` now take captured Gradle services
(`ConfigurationContainer`, `DependencyHandler`, `Function<String, String>`
propertyLookup) instead of a `Project`. This matches the
`captureProjectServices` pattern already used by `ExtractDependenciesTask`. The
result is a `BomManagedVersions` instance holding only a `Map<String, String>`
of overrides - pure data, no `Project` reference - which is consumed by
per-configuration `eachDependency` closures that survive configuration-cache
serialisation cleanly.
The `afterEvaluate` trigger is preserved because it is the project's
established pattern (13 uses across `GrailsGradlePlugin` alone, all addressing
the same late-binding-to-the-user's-declarations need). Empirical verification
with `./gradlew --configuration-cache :grails-gradle-plugins:test` shows
**zero** CC warnings originating from this plugin or `BomManagedVersions` - the
7 CC problems reported are all in pre-existing Jar tasks and
`test-config.gradle`, none from this PR.
`Project`-accepting convenience overloads on `BomManagedVersions.resolve()`
are retained for tests and ad-hoc usage.
### 3. Drop Spring DM from `build-logic/docs-core` (the last consumer)
`ExtractDependenciesTask` was the only file in the entire repo still
importing `io.spring.gradle.dependencymanagement.org.apache.maven.model.*`
(Spring DM's shaded Maven model). The `MavenXpp3Reader` + `Model` usage is
replaced with JDK `DocumentBuilderFactory` POM parsing following the same
XXE-hardened pattern `BomManagedVersions` established
(`setXIncludeAware(false)`, `disallow-doctype-decl`, external entities off). A
private `ManagedDependency` data class replaces the four shaded Maven model
fields the task actually used.
With `ExtractDependenciesTask` off Spring DM, the `implementation
'org.springframework.boot:spring-boot-gradle-plugin'` dependency in
`build-logic/docs-core/build.gradle` is no longer required - it was only there
to drag Spring DM onto the build-logic classpath via its transitive. Dropped.
**After this commit the repository has zero compile-time, runtime, or
build-classpath dependency on `io.spring.gradle:dependency-management-plugin`
in any form.**
### Consistency audit (religious adherence to existing project patterns)
Verified each new pattern lines up with what the rest of the codebase
already does:
| Pattern | Pre-existing uses | Where we use it |
|---|---|---|
| `project.afterEvaluate { ... }` for late-binding | 13 in
`GrailsGradlePlugin` | `applyGrailsBom()`, `BomPropertyOverridesPlugin.apply()`
|
| `project.objects.property(...).convention(...)` for extension fields |
`GrailsExtension.indy`, `GrailsExtension.preserveParameterNames`,
`BomPropertyOverridesExtension.autoDetect`/`boms` |
`GrailsExtension.autoApplyBom` |
| `project.extensions.create(...)` for plugin extensions | 2 existing |
`BomPropertyOverridesPlugin.apply()` |
| `configurations.detachedConfiguration(...)` for ad-hoc POM resolution |
`GrailsDependencyValidatorPlugin`, `ExtractDependenciesTask` |
`BomManagedVersions.resolvePomFile()` |
| JDK `DocumentBuilderFactory` POM parsing with XXE hardening |
`BomManagedVersions.parseXml()` (this PR) |
`ExtractDependenciesTask.parsePom()` (same JDK pattern, same XXE flags) |
| `captureProjectServices`-style service capture |
`ExtractDependenciesTask`, `TestPhasesGradlePlugin` |
`BomManagedVersions.resolve(ConfigurationContainer, DependencyHandler, ...)` |
### Verification
```
./gradlew :grails-gradle-plugins:build
-> BUILD SUCCESSFUL, 31 tests pass
(was 30; +1 AutoApplyBomSpec)
codenarc clean, validatePlugins clean
./gradlew :grails-base-bom:extractConstraints
-> BUILD SUCCESSFUL
grails-bom-constraints.adoc produced (184.9 KB)
identical structure to pre-refactor output
./gradlew --configuration-cache :grails-gradle-plugins:test
-> 0 CC warnings from BOM code
(7 pre-existing warnings in Jar tasks + test-config.gradle, none from
this PR)
git grep "io.spring.gradle.dependencymanagement" -> 0 matches
git grep "spring-boot-gradle-plugin" -> 0 matches outside this
commit's removal
```
PR description rewritten to describe the final state and justify the custom
code.
All 28 review threads are now resolved (15 in the original review pass + 13
in the three follow-up passes).
--
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]
