This is an automated email from the ASF dual-hosted git repository.
borinquenkid pushed a commit to branch fix/plugin-dependency-diagnostics
in repository https://gitbox.apache.org/repos/asf/grails-core.git
The following commit(s) were added to
refs/heads/fix/plugin-dependency-diagnostics by this push:
new 9e73262146 Address review feedback on plugin dependency diagnostics
9e73262146 is described below
commit 9e732621461bec6e48ecd93efb17cd3186f6b6eb
Author: Walter Duque de Estrada <[email protected]>
AuthorDate: Mon Jul 27 12:44:18 2026 -0500
Address review feedback on plugin dependency diagnostics
- Revert the unresolved-dependency log from WARN back to ERROR, per
jdaugherty and davydotcom (davydotcom revised an initial approval to
CHANGES_REQUESTED specifically for this): unresolved dependencies are
a real load failure - the plugin already lands in failedPlugins - and
the prior code logged it at ERROR. WARN risked being filtered out of
environments that only surface errors.
- Fix logUnresolvedDependencies() misclassifying a dependency as
"missing" whenever it isn't yet registered, even when the dependency
plugin actually exists but itself failed to load or is still waiting
in the delayed-load queue (flagged independently by Copilot's inline
review and bito-code-review's bot comment; verified by tracing
loadDelayedPlugins()'s processing order). Now distinguishes three
cases: registered with an incompatible version (unchanged), found in
failedPlugins ("failed to load"), found in delayedLoadPlugins ("is
still pending load"), and only "is missing" when none of the above.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
---
.../core/plugins/DefaultPluginDiscovery.java | 39 +++++++++++---
.../grails/core/plugins/PluginDiscoverySpec.groovy | 61 +++++++++++++++++++---
2 files changed, 86 insertions(+), 14 deletions(-)
diff --git
a/grails-core/src/main/groovy/org/apache/grails/core/plugins/DefaultPluginDiscovery.java
b/grails-core/src/main/groovy/org/apache/grails/core/plugins/DefaultPluginDiscovery.java
index c6d33d8689..f3f25731dc 100644
---
a/grails-core/src/main/groovy/org/apache/grails/core/plugins/DefaultPluginDiscovery.java
+++
b/grails-core/src/main/groovy/org/apache/grails/core/plugins/DefaultPluginDiscovery.java
@@ -429,18 +429,29 @@ public class DefaultPluginDiscovery implements
PluginDiscovery {
for (var name : plugin.getDependsOnNames()) {
var requiredVersion =
plugin.getMetadata().getDependentVersion(name);
var dependency = findPlugin(name);
- if (dependency == null) {
+ if (dependency != null) {
+ if
(!GrailsVersionUtils.isValidVersion(dependency.getPluginVersion(),
requiredVersion)) {
+ unresolvedDependencies.add(
+ "dependency [" + name + "] has version [" +
dependency.getPluginVersion() +
+ "] but requires [" + requiredVersion + "]"
+ );
+ }
+ } else if (getFailedPlugin(name) != null) {
unresolvedDependencies.add(
- "dependency [" + name + "] with required version [" +
requiredVersion + "] is missing"
+ "dependency [" + name + "] with required version [" +
requiredVersion + "] failed to load"
+ );
+ } else if (isDelayed(name)) {
+ unresolvedDependencies.add(
+ "dependency [" + name + "] with required version [" +
requiredVersion +
+ "] is still pending load"
);
- } else if
(!GrailsVersionUtils.isValidVersion(dependency.getPluginVersion(),
requiredVersion)) {
+ } else {
unresolvedDependencies.add(
- "dependency [" + name + "] has version [" +
dependency.getPluginVersion() +
- "] but requires [" + requiredVersion + "]"
+ "dependency [" + name + "] with required version [" +
requiredVersion + "] is missing"
);
}
}
- LOG.warn(
+ LOG.error(
"Grails plug-in [{}] with version [{}] cannot be loaded: {}",
plugin.getName(),
plugin.getPluginVersion(),
@@ -448,6 +459,22 @@ public class DefaultPluginDiscovery implements
PluginDiscovery {
);
}
+ /**
+ * Checks whether a plugin of the given name is still waiting to be
resolved.
+ *
+ * @param name the plugin name to look for
+ * @return {@code true} if a plugin with this name is currently in {@link
#delayedLoadPlugins}
+ */
+ private boolean isDelayed(String name) {
+ var normalizedName = PluginUtils.normalizePluginName(name);
+ for (var delayed : delayedLoadPlugins) {
+ if
(PluginUtils.normalizePluginName(delayed.getName()).equals(normalizedName)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
/**
* Checks whether the first plugin is dependent on the second plugin.
*
diff --git
a/grails-core/src/test/groovy/org/apache/grails/core/plugins/PluginDiscoverySpec.groovy
b/grails-core/src/test/groovy/org/apache/grails/core/plugins/PluginDiscoverySpec.groovy
index 893beb5ed7..a809720e0c 100644
---
a/grails-core/src/test/groovy/org/apache/grails/core/plugins/PluginDiscoverySpec.groovy
+++
b/grails-core/src/test/groovy/org/apache/grails/core/plugins/PluginDiscoverySpec.groovy
@@ -373,7 +373,7 @@ class RepeatedProbeGrailsPlugin {
System.setErr(originalErr)
}
- def 'reports every missing or incompatible plugin dependency at WARN'() {
+ def 'reports every missing or incompatible plugin dependency at ERROR'() {
given: 'a discovery bean with one available and one unresolved plugin'
def gcl = new GroovyClassLoader()
def availableDependencyClass = gcl.parseClass('''
@@ -405,13 +405,13 @@ class UnresolvedDependenciesGrailsPlugin {
!discovery.hasPlugin('unresolvedDependencies')
discovery.getPluginsInLoadOrder()*.name == ['availableDependency']
- and: 'one concise warning identifies the failed plugin and every
unresolved dependency'
- def warnings = captured.toString().readLines().findAll {
it.contains('unresolvedDependencies') }
- warnings.size() == 1
- warnings[0].contains('WARN')
- warnings[0].contains('Grails plug-in [unresolvedDependencies] with
version [4.0.0] cannot be loaded')
- warnings[0].contains('dependency [missingDependency] with required
version [1.0.0] is missing')
- warnings[0].contains('dependency [availableDependency] has version
[1.0.0] but requires [2.0.0]')
+ and: 'one concise message at ERROR identifies the failed plugin and
every unresolved dependency'
+ def errors = captured.toString().readLines().findAll {
it.contains('unresolvedDependencies') }
+ errors.size() == 1
+ errors[0].contains('ERROR')
+ errors[0].contains('Grails plug-in [unresolvedDependencies] with
version [4.0.0] cannot be loaded')
+ errors[0].contains('dependency [missingDependency] with required
version [1.0.0] is missing')
+ errors[0].contains('dependency [availableDependency] has version
[1.0.0] but requires [2.0.0]')
and: 'normal verbosity does not include a stack trace'
!captured.toString().contains('at
org.apache.grails.core.plugins.DefaultPluginDiscovery')
@@ -419,6 +419,51 @@ class UnresolvedDependenciesGrailsPlugin {
cleanup:
System.setErr(originalErr)
}
+
+ def 'reports a dependency that failed to load as failed rather than
missing'() {
+ given: 'a plugin whose only dependency itself fails to load due to a
genuinely missing dependency'
+ def gcl = new GroovyClassLoader()
+ def failingDependencyClass = gcl.parseClass('''
+class FailingDependencyGrailsPlugin {
+ def version = "1.0.0"
+ def dependsOn = [reallyMissing: "1.0.0"]
+}
+''')
+ def dependentClass = gcl.parseClass('''
+class DependentOnFailedGrailsPlugin {
+ def version = "1.0.0"
+ def dependsOn = [failingDependency: "1.0.0"]
+}
+''')
+ def discovery = new DefaultPluginDiscovery(
+ [failingDependencyClass, dependentClass] as Class<?>[]
+ )
+ discovery.loadPluginsFromClasspath = false
+
+ and: 'standard error is captured to observe slf4j-simple output'
+ def originalErr = System.err
+ def captured = new ByteArrayOutputStream()
+ System.setErr(new PrintStream(captured, true))
+
+ when:
+ discovery.init(new StandardEnvironment())
+
+ then: 'both plugins end up failed'
+ discovery.hasFailedPlugin('failingDependency')
+ discovery.hasFailedPlugin('dependentOnFailed')
+
+ and: 'the missing root cause is reported as missing'
+ def rootCause = captured.toString().readLines().find {
it.contains('[failingDependency]') }
+ rootCause.contains('dependency [reallyMissing] with required version
[1.0.0] is missing')
+
+ and: 'the plugin depending on the failed plugin reports it as failed,
not missing'
+ def dependent = captured.toString().readLines().find {
it.contains('[dependentOnFailed]') }
+ dependent.contains('dependency [failingDependency] with required
version [1.0.0] failed to load')
+ !dependent.contains('is missing')
+
+ cleanup:
+ System.setErr(originalErr)
+ }
}
// Test fixture plugin classes for GrailsPluginDiscoverySpec
