jamesfredley opened a new pull request, #16073:
URL: https://github.com/apache/grails-core/pull/16073
## Summary
`grails-test-examples/spring-dependency-management` has never actually
tested the commit it runs on, and it takes the whole build down whenever
`projectVersion` is bumped to a version that has not been published yet. This
makes it resolve the BOM produced by the current build.
Reviewers: the interesting parts are `build.gradle` and
`BomPropertyOverridesPlugin.groovy`. The rest follows from those.
## Background
The example deliberately opts out of the native platform injection and
imports the BOM the old way, to keep coverage for applications migrated from
Grails 7:
```groovy
grails { bom = null }
apply plugin: 'io.spring.dependency-management'
dependencyManagement {
imports { mavenBom "org.apache.grails:grails-bom:${projectVersion}" }
}
```
`io.spring.dependency-management` resolves that import as an **artifact-only
`@pom` request inside its own detached configuration**. Detached configurations
never see the `dependencySubstitution` rules in
`gradle/functional-test-config.gradle`, which is what maps
`org.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. Setting
`guava.version` to `33.5.0-jre` in `dependencies.gradle` and asking Spring DM
what it manages:
```
com.google.guava:guava 33.6.0-jre <-- the published BOM, not the local
edit
```
So any PR touching `dependencies.gradle` went unverified by this example,
and on `8.0.x` the 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**:
```
Could not find org.apache.groovy:groovy:.
Could not find org.springframework:spring-core:.
...
```
On a freshly created release branch the bumped version has never been
published, so this fired immediately and took roughly 30 CI jobs down.
`publish` is gated behind `build`, so CI could not publish the snapshot that
would have fixed it - the branch could not bootstrap itself. This happened on
`8.1.x` and was worked around with the temporary `-PbomSnapshotNotPublished`
flag added in 57883c41e4, which this PR removes.
## What changed
**1. Stage the in-commit BOM poms (`build.gradle`)**
Generate the poms for `grails-base-bom`, `grails-bom`,
`grails-hibernate5-bom` and `grails-hibernate7-bom` and write them into
`.gradle/local-boms` during root configuration.
Configuration time is not a stylistic choice. Spring DM resolves its
detached import **while Gradle is still computing the task graph** - the
failure surfaces as `Could not determine the dependencies of task
':...:compileGroovy'` - so no `dependsOn` on a publish or pom task can ever run
early enough. Two details worth reviewing:
- The directory is `.gradle/local-boms`, deliberately **outside `build/`**.
In a combined `./gradlew clean <task>` invocation - including this repo's own
required `clean aggregateViolations` - `clean` would otherwise delete the poms
after configuration wrote them and before the example resolved them. `.gradle/`
is already gitignored.
- `grails-base-bom` is evaluated **last**. It builds its constraints by
walking the other projects and treats anything not yet carrying `java-platform`
as an ordinary published module, so evaluating it first made it adopt the
sibling BOMs as managed dependencies and bake that into its published pom.
**2. Serve them exclusively (`GrailsRepoSettingsPlugin.groovy`)**
`RepositoriesMode.FAIL_ON_PROJECT_REPOS` is set, so the repository has to be
declared at settings level. It uses `exclusiveContent` rather than a plain
content filter on purpose: with a plain filter, a missing local pom falls
through to the remote and silently restores the original stale-BOM behaviour,
which is precisely the bug being fixed and it fails invisibly. Scoped by a
directory check so only the grails-core root build is affected and other
consumers of this shared settings plugin are untouched.
**3. Fix project-platform detection (`BomPropertyOverridesPlugin.groovy`)**
This was the actual root cause of the last four coordinates that would not
resolve. The plugin was requesting
```
grails.core.ROOT:grails-hibernate5-bom:unspecified
grails.core.ROOT:grails-hibernate7-bom:unspecified
```
- the root project name as the group, and no version - for platforms that
are projects in this build. Those cannot resolve, so `hibernate-core-jakarta`
and the three sitemesh coordinates were left unmanaged. Project platforms are
now skipped rather than resolved as external modules.
**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`)**
`-PbomSnapshotNotPublished` is gone and the example is included
unconditionally, including during reproducible release builds - the original
`isReproducibleBuild` exclusion existed for this same "BOM not published yet"
reason and is no longer needed.
The example's own `build.gradle` is **unchanged**. It still imports exactly
one BOM.
## Verification
| Check | Result |
|---|---|
| Cold build at `-PprojectVersion=9.9.9-SNAPSHOT`, a version that has never
existed | exit 0 |
| Cold build at the default `8.0.0-SNAPSHOT` | exit 0 - proves the remote
BOM is no longer needed at all |
| `./gradlew clean :...:compileGroovy` combined, cold | exit 0 |
| `guava.version=33.5.0-jre` → Spring DM reports | `com.google.guava:guava
33.5.0-jre` (was `33.6.0-jre`) |
| Generated `grails-base-bom` pom contains sibling BOMs | no - confirmed
clean |
| `:grails-gradle-plugins:test --tests "*BomPropertyOverridesPluginSpec*"` |
BUILD SUCCESSFUL, 8 tests passed |
"Cold" means `.gradle/local-boms` deleted first, so nothing is inherited
from a previous run.
## Known follow-ups
Deliberately out of scope, raised in review and worth tracking separately:
1. Pom generation runs during every root configuration and calls
`GenerateMavenPom.doGenerate()` directly rather than through a supported API.
It works and the cost is small, but it is lifecycle-fragile and would need
revisiting before the configuration cache is enabled.
2. `BomPropertyOverridesPlugin` now skips **all** `ProjectDependency`
platforms, which is broader than its documented "auto-detects declared
platforms" behaviour. Consumers relying on project platforms must declare them
explicitly with `bom(...)`. This should either be narrowed or documented, with
functional coverage.
## Merge-up
Targets `8.0.x`. Once merged, `8.1.x` and `9.0.x` need
`bomSnapshotNotPublished=true` deleted from `gradle.properties` - it becomes a
no-op immediately and a dead property after that.
--
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]
