jdaugherty commented on code in PR #15467:
URL: https://github.com/apache/grails-core/pull/15467#discussion_r3348848272


##########
dependencies.gradle:
##########
@@ -38,6 +38,10 @@ ext {
             'jline2.version'                : '2.14.6',
             'jna.version'                   : '5.18.1',
             'jquery.version'                : '3.7.1',
+            // maven-model is used by the docs-core ExtractDependenciesTask to 
parse BOM POMs

Review Comment:
   Remove the comment



##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsDependencyValidatorPlugin.groovy:
##########
@@ -159,19 +161,49 @@ class GrailsDependencyValidatorPlugin implements 
Plugin<Project> {
 
     /**
      * Scans the project's configurations to find which BOM project is in use.
+     *
+     * <p>When multiple known BOMs are declared on the same project (for 
example,
+     * the {@code grails-app} plugin auto-injects {@code platform(grails-bom)} 
on

Review Comment:
   I don't believe this is possible now with the current bom design - we should 
only be using one bom.



##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy:
##########
@@ -478,6 +552,14 @@ ${importStatements}
             return
         }
 
+        // The Grails Gradle Plugin injects a regular platform(grails-bom) 
into each
+        // declarable configuration via applyGrailsBom(), excluding 
code-quality and
+        // annotation-processor classpaths (see isExcludedFromBomPlatform). 
For Micronaut
+        // projects the user must additionally declare an enforcedPlatform on 
a Micronaut BOM

Review Comment:
   The original design was to *only* have one bom applied, I believe this PR is 
out of date.



##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/bom/BomManagedVersions.groovy:
##########
@@ -0,0 +1,504 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.grails.gradle.plugin.bom
+
+import java.util.function.Function
+
+import groovy.transform.CompileStatic
+import org.gradle.api.Project
+import org.gradle.api.artifacts.ConfigurationContainer
+import org.gradle.api.artifacts.dsl.DependencyHandler
+import org.gradle.api.logging.Logger
+import org.gradle.api.logging.Logging
+import org.w3c.dom.Document
+import org.w3c.dom.Element
+
+import javax.xml.parsers.DocumentBuilderFactory
+
+/**
+ * Lightweight replacement for the Spring Dependency Management plugin's
+ * version property override feature.
+ *
+ * <p>Parses BOM POM files to determine the version every managed artifact
+ * resolves to, both with the BOM's default {@code <properties>} values and
+ * with the project's overrides applied (via {@code ext['property.name']} in
+ * {@code build.gradle} or via {@code gradle.properties}). Any artifact whose
+ * effective version differs from the BOM default becomes a version 
override.</p>
+ *
+ * <p>Overrides are applied as <strong>strict</strong> dependency constraints
+ * (see {@link #applyTo(DependencyHandler, String)}). A strict constraint wins
+ * over the {@code require} constraints contributed by Gradle's native
+ * {@code platform()} mechanism, so an override is honored even when it
+ * <em>downgrades</em> a managed version - a plain
+ * {@code ResolutionStrategy.eachDependency()} / {@code useVersion()} hook
+ * would lose to the platform's higher version during conflict resolution.</p>
+ *
+ * <p>Because the effective version is computed by re-resolving imported
+ * ({@code <scope>import</scope>}) BOMs with the project's property overrides
+ * applied, overriding a property that selects an imported BOM's version
+ * (for example {@code spring-boot.version}) re-imports that BOM and pulls in
+ * its updated managed-dependency set.</p>
+ *
+ * <p>Gradle's native {@code platform()} mechanism handles the base BOM import
+ * and default version management. This class only adds the one feature Gradle
+ * lacks: property-based version customization
+ * (see <a href="https://github.com/gradle/gradle/issues/9160";>Gradle 
#9160</a>).</p>
+ *
+ * <p>This is the underlying utility used by the
+ * {@code org.apache.grails.gradle.bom-property-overrides} plugin
+ * (registered in {@code grails-gradle-plugins}). It is BOM-agnostic and
+ * can be used directly with any BOM that follows the Maven
+ * {@code <properties>} convention for managed versions.</p>
+ *
+ * @since 8.0
+ */
+@CompileStatic
+class BomManagedVersions {
+
+    private static final Logger LOG = Logging.getLogger(BomManagedVersions)
+    private static final int MAX_PROPERTY_INTERPOLATION_DEPTH = 10
+
+    /** A property resolver that never overrides anything (BOM defaults only). 
*/
+    private static final Function<String, String> NO_OVERRIDES = { String name 
-> null } as Function<String, String>
+
+    private final Map<String, String> versionOverrides = new LinkedHashMap<>()
+
+    /**
+     * Resolves a single BOM via captured Gradle services rather than a
+     * {@link Project} reference. Preferred for config-cache discipline:
+     * callers capture services once (typically inside a single
+     * {@code afterEvaluate} block) and never leak a {@link Project}
+     * reference into the override map that lives on past configuration time.
+     *
+     * @param configurations the project's configuration container, captured 
at apply/afterEvaluate time
+     * @param dependencies the project's dependency handler, captured at 
apply/afterEvaluate time
+     * @param propertyLookup function returning the project's property value 
as a String, or {@code null} if unset
+     * @param bomCoordinates the BOM coordinates in {@code 
group:artifact:version} format
+     * @return a BomManagedVersions instance containing any version overrides 
to apply
+     */
+    static BomManagedVersions resolve(ConfigurationContainer configurations,
+                                      DependencyHandler dependencies,
+                                      Function<String, String> propertyLookup,
+                                      String bomCoordinates) {
+        resolve(configurations, dependencies, propertyLookup, [bomCoordinates])
+    }
+
+    /**
+     * Resolves multiple BOMs via captured Gradle services. The result is a
+     * plain data carrier (a {@code Map<String, String>} of version overrides
+     * inside {@link BomManagedVersions}) that holds no {@link Project}
+     * reference, so it can be safely captured by per-configuration
+     * constraint declarations and survive configuration-cache serialization.
+     *
+     * <p>The override set is computed as the difference between two 
resolutions
+     * of the BOM tree: one using the BOM's default property values, and one
+     * using the project's property overrides. Any managed artifact whose
+     * effective version differs from its default version is recorded as an
+     * override.</p>
+     *
+     * @param configurations the project's configuration container, captured 
at apply/afterEvaluate time
+     * @param dependencies the project's dependency handler, captured at 
apply/afterEvaluate time
+     * @param propertyLookup function returning the project's property value 
as a String, or {@code null} if unset
+     * @param bomCoordinatesList list of BOM coordinates in {@code 
group:artifact:version} format
+     * @return a BomManagedVersions instance containing any version overrides 
to apply
+     */
+    static BomManagedVersions resolve(ConfigurationContainer configurations,
+                                      DependencyHandler dependencies,
+                                      Function<String, String> propertyLookup,
+                                      Collection<String> bomCoordinatesList) {
+        def instance = new BomManagedVersions()
+
+        def defaultVersions = computeManagedVersions(
+                configurations, dependencies, bomCoordinatesList, NO_OVERRIDES)
+        def effectiveVersions = computeManagedVersions(
+                configurations, dependencies, bomCoordinatesList, 
propertyLookup)
+
+        for (def entry : effectiveVersions.entrySet()) {
+            def artifactKey = entry.key
+            def effectiveVersion = entry.value
+            def defaultVersion = defaultVersions.get(artifactKey)
+
+            if (effectiveVersion != null && effectiveVersion != 
defaultVersion) {
+                instance.versionOverrides.put(artifactKey, effectiveVersion)
+                LOG.info(
+                    'BOM version override: {} = {} (BOM default: {})',
+                    artifactKey, effectiveVersion, defaultVersion ?: 'unknown'
+                )
+            }
+        }
+
+        if (!instance.versionOverrides.isEmpty()) {
+            LOG.lifecycle(
+                'BOM property overrides: {} version override(s) will be 
applied',
+                instance.versionOverrides.size()
+            )
+        }
+
+        instance
+    }
+
+    /**
+     * Convenience overload that captures services from the given {@link 
Project}.
+     * Production callers should prefer the services-based overload above so 
the
+     * resolve path never sees a {@link Project} reference. This overload is
+     * primarily useful for tests and ad-hoc usage.
+     *
+     * @param project the Gradle project (services are extracted at call time)
+     * @param bomCoordinates the BOM coordinates in {@code 
group:artifact:version} format
+     */
+    static BomManagedVersions resolve(Project project, String bomCoordinates) {
+        resolve(project, [bomCoordinates])
+    }
+
+    /**
+     * Convenience overload that captures services from the given {@link 
Project}.
+     * Production callers should prefer the services-based overload above so 
the
+     * resolve path never sees a {@link Project} reference. This overload is
+     * primarily useful for tests and ad-hoc usage.
+     *
+     * @param project the Gradle project (services are extracted at call time)
+     * @param bomCoordinatesList list of BOM coordinates in {@code 
group:artifact:version} format
+     */
+    static BomManagedVersions resolve(Project project, Collection<String> 
bomCoordinatesList) {
+        resolve(
+            project.configurations,
+            project.dependencies,
+            { String name -> project.hasProperty(name) ? 
project.property(name)?.toString() : null } as Function<String, String>,
+            bomCoordinatesList
+        )
+    }
+
+    /**
+     * Applies the detected version overrides to the given configuration as
+     * strict dependency constraints.
+     *
+     * <p>Strict constraints are used deliberately: a plain {@code platform()}
+     * contributes {@code require} constraints, and a soft override (e.g.
+     * {@code useVersion()}) would lose to a higher {@code require} version
+     * during conflict resolution. A strict constraint overrides {@code 
require},
+     * so the project's chosen version wins in both directions (upgrade and
+     * downgrade).</p>
+     *
+     * @param dependencies the project's dependency handler
+     * @param configurationName the name of the configuration to add 
constraints to
+     */
+    void applyTo(DependencyHandler dependencies, String configurationName) {
+        if (versionOverrides.isEmpty()) {
+            return
+        }
+
+        versionOverrides.each { String coordinate, String version ->
+            dependencies.constraints.add(configurationName, coordinate) {
+                it.version { it.strictly(version) }
+                it.because('BOM version override via project property')
+            }
+        }
+    }
+
+    /**
+     * Returns whether any version overrides were detected.
+     */
+    boolean hasOverrides() {
+        !versionOverrides.isEmpty()
+    }
+
+    /**
+     * Returns an unmodifiable view of the version overrides.
+     * Keys are {@code group:artifact}, values are the override version 
strings.
+     */
+    Map<String, String> getOverrides() {
+        Collections.unmodifiableMap(versionOverrides)
+    }
+
+    /**
+     * Parses a BOM POM file and extracts the property-to-artifact mapping.
+     * This method does not follow imported BOMs recursively - it only 
processes
+     * the given file. Intended for testing and direct POM inspection.
+     *
+     * @param pomFile the BOM POM file to parse
+     * @param bomProperties output map to receive property name to default 
value mappings
+     * @param propertyToArtifacts output map to receive property name to 
artifact coordinate mappings
+     */
+    static void parseBomFile(File pomFile, Map<String, String> bomProperties, 
Map<String, List<String>> propertyToArtifacts) {
+        def doc = parseXml(pomFile)

Review Comment:
   We should use the maven model in this file.



##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsExtension.groovy:
##########
@@ -121,6 +146,32 @@ class GrailsExtension {
      */
     final Property<Boolean> preserveParameterNames
 
+    /**
+     * Whether the Grails Gradle plugin should automatically apply the {@code 
grails-bom}
+     * as a Gradle {@code platform()} on every declarable project configuration
+     * (and apply the {@code org.apache.grails.gradle.bom-property-overrides} 
plugin
+     * for property-based version overrides).
+     *
+     * <p>Defaults to {@code true}, which matches the behaviour of every 
Grails 7 release:
+     * the BOM is always applied so that the framework's curated 
managed-dependency set
+     * is the source of truth for the application.</p>
+     *
+     * <p>Disable this only when you intentionally want to manage Grails 
dependencies
+     * yourself - for example, when consuming Grails modules from a different 
curated
+     * platform and you need to declare the BOM by hand (and apply
+     * {@code org.apache.grails.gradle.bom-property-overrides} explicitly if 
you still
+     * want {@code gradle.properties} / {@code ext['...']} overrides).</p>
+     *
+     * <pre>
+     * grails {
+     *     autoApplyBom = false

Review Comment:
   Because the boms are split betwen hibernate 5 & 7, I don't think we should 
do this.



##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy:
##########
@@ -367,36 +366,111 @@ ${importStatements}
 
     protected void applyDefaultPlugins(Project project) {
         applySpringBootPlugin(project)
+        applyGrailsBom(project)
+    }
 
+    /**
+     * Applies the Grails BOM as a Gradle platform and enables property-based
+     * version overrides via the standalone
+     * {@code org.apache.grails.gradle.bom-property-overrides} plugin.
+     *
+     * <p>This replaces the Spring Dependency Management plugin with two
+     * orthogonal pieces:</p>
+     * <ol>
+     *   <li><strong>BOM import</strong>: {@code grails-bom} is added as a
+     *       Gradle {@code platform()} dependency on every declarable
+     *       configuration, mirroring the global behaviour Spring DM provided
+     *       via {@code configurations.all() + 
resolutionStrategy.eachDependency()}.</li>
+     *   <li><strong>Property overrides</strong>: the BOM-agnostic
+     *       {@link BomPropertyOverridesPlugin} reads the BOM's
+     *       {@code <properties>} block and applies any project-level
+     *       overrides via Gradle's
+     *       {@code ResolutionStrategy.eachDependency()}.</li>
+     * </ol>
+     *
+     * <p>Usage: to override a version managed by the Grails or Spring Boot 
BOM, set the
+     * corresponding property in {@code gradle.properties} or {@code 
build.gradle}:</p>
+     * <pre>
+     * // gradle.properties
+     * slf4j.version=1.7.36
+     *
+     * // or build.gradle
+     * ext['slf4j.version'] = '1.7.36'
+     * </pre>
+     *
+     * @see BomPropertyOverridesPlugin
+     * @since 8.0
+     */
+    protected void applyGrailsBom(Project project) {
+        // Ensure the developmentOnly configuration exists. Spring Boot's 
plugin
+        // normally creates this, but using maybeCreate guarantees it is 
available
+        // even if plugin ordering changes or Spring Boot is not applied. We do
+        // this outside afterEvaluate so that other plugins applied during the
+        // same configuration phase can rely on the configuration existing.
+        project.configurations.maybeCreate('developmentOnly')
+
+        // The opt-out flag `grails { autoApplyBom = false }` is set in the 
user's
+        // build.gradle, which runs AFTER plugin apply. We therefore wait until
+        // afterEvaluate to read the flag and apply the BOM accordingly. By 
that
+        // point all declarable configurations exist (java-base creates them
+        // during apply), so iterating them eagerly via .each is sufficient -
+        // any plugin that adds a configuration later is responsible for
+        // declaring its own BOM coordination if it needs it.
         project.afterEvaluate {
-            GrailsExtension ge = project.extensions.getByType(GrailsExtension)
-            if (ge.springDependencyManagement) {
-                Plugin dependencyManagementPlugin = 
project.plugins.findPlugin(DependencyManagementPlugin)
-                if (dependencyManagementPlugin == null) {
-                    project.plugins.apply(DependencyManagementPlugin)
-                }
-
-                DependencyManagementExtension dme = 
project.extensions.findByType(DependencyManagementExtension)
+            def grailsExtension = 
project.extensions.findByType(GrailsExtension)
+            boolean autoApply = grailsExtension == null || 
grailsExtension.autoApplyBom.getOrElse(true)
+            if (!autoApply) {
+                project.logger.info(
+                    'grails.autoApplyBom is false; skipping automatic 
application of platform(grails-bom) and bom-property-overrides plugin for 
project {}',
+                    project.path
+                )
+                return
+            }
 
-                applyBomImport(dme, project)
+            def grailsVersion = (project.findProperty('grailsVersion') ?: 
BuildSettings.grailsVersion) as String
+            def bomCoordinates = 
"org.apache.grails:grails-bom:${grailsVersion}" as String

Review Comment:
   We can't assume this.  If we're really going to set the bom, we should 
change the boolean to the bom name and then default it.  If it's null, we 
simply don't apply.



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