jamesfredley commented on PR #16073:
URL: https://github.com/apache/grails-core/pull/16073#issuecomment-5137913854

   ## Converting to draft - this drops all `*-cli` constraints from the BOM
   
   A further automated review pass caught a regression in this PR that I have 
now confirmed empirically. Flagging it here rather than leaving the PR looking 
ready.
   
   **The defect.** `build.gradle` calls `GenerateMavenPom.doGenerate()` during 
root-project evaluation. That realizes each BOM's `api` configuration before 
ordinary subprojects have been evaluated. 
`gradle/cli-companion-bom-constraints.gradle` deliberately discovers 
`cliArtifactId` inside `withDependencies`, i.e. only *after* the producing 
projects are evaluated - so forcing realization early permanently omits every 
CLI companion constraint.
   
   **Evidence.** Comparing the currently published BOM with the one this branch 
generates:
   
   | BOM | `<artifactId>` entries | `grails-*-cli` constraints |
   |---|---|---|
   | Published `grails-bom:8.0.0-SNAPSHOT` | 111 | 9 |
   | Generated on this branch | 102 | **0** |
   
   The 9 missing entries are exactly:
   
   ```
   grails-core-cli
   grails-data-hibernate5-cli
   grails-data-hibernate5-dbmigration-cli
   grails-data-hibernate7-cli
   grails-data-hibernate7-dbmigration-cli
   grails-scaffolding-cli
   grails-spring-security-cli
   grails-spring-security-oauth2-cli
   grails-web-url-mappings-cli
   ```
   
   This is not limited to the throwaway staged pom - the same premature 
realization affects the BOM subsequently **published** from that build, which 
would leave auto-provisioned CLI dependencies without managed versions for 
consumers. That is a worse outcome than the bug this PR set out to fix, so it 
must not merge in this state.
   
   **Second finding, not yet verified.** When the Micronaut island is active 
(`-PincludeMicronautProjects`, or JDK 25), the three Micronaut BOM projects are 
absent from the evaluation list in `build.gradle`, so they have not applied 
`java-platform` when `grails-base-bom` scans subprojects and will be 
misclassified as ordinary modules - the same class of bug as the base-BOM 
ordering issue already fixed in this branch, just for a different set of 
projects.
   
   **Where that leaves the approach.** The diagnosis and the rest of the change 
still stand: the detached-configuration import genuinely cannot see project 
substitution, the example genuinely was validating the last-published BOM 
rather than the commit, and staging the in-commit poms genuinely fixes both. 
What is wrong is *when* the poms are generated. The fix needs a generation 
point that is late enough for `withDependencies` discovery to have run, but 
still early enough for Spring DM's import - which resolves during task-graph 
computation. That is a narrow window and worth designing deliberately rather 
than patching.
   
   I would rather leave this visible and correct than merge something that 
quietly ships an incomplete BOM.
   


-- 
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]

Reply via email to