From 9e30fb08e5fa64ba838259bfff1e69bd2c10cc25 Mon Sep 17 00:00:00 2001 From: James Fredley Date: Fri, 17 Jul 2026 13:51:19 -0400 Subject: [PATCH 1/2] Improve unresolved plugin dependency diagnostics Assisted-by: opencode:gpt-5.6-sol --- .../core/plugins/DefaultPluginDiscovery.java | 30 ++++++++++-- .../core/plugins/PluginDiscoverySpec.groovy | 47 +++++++++++++++++++ 2 files changed, 72 insertions(+), 5 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 dc85b2d8643..c6d33d86894 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 @@ -418,16 +418,36 @@ private void loadDelayedPlugins() { delayedLoadPlugins.add(plugin); } else { failedPlugins.put(plugin.getName(), plugin); - LOG.error( - "ERROR: Plugin [{}] cannot be loaded because its dependencies [{}}] cannot be resolved", - plugin.getName(), - plugin.getDependsOnNames() - ); + logUnresolvedDependencies(plugin); } } } } + private void logUnresolvedDependencies(PluginInfo plugin) { + var unresolvedDependencies = new ArrayList(); + for (var name : plugin.getDependsOnNames()) { + var requiredVersion = plugin.getMetadata().getDependentVersion(name); + var dependency = findPlugin(name); + if (dependency == null) { + unresolvedDependencies.add( + "dependency [" + name + "] with required version [" + requiredVersion + "] is missing" + ); + } else if (!GrailsVersionUtils.isValidVersion(dependency.getPluginVersion(), requiredVersion)) { + unresolvedDependencies.add( + "dependency [" + name + "] has version [" + dependency.getPluginVersion() + + "] but requires [" + requiredVersion + "]" + ); + } + } + LOG.warn( + "Grails plug-in [{}] with version [{}] cannot be loaded: {}", + plugin.getName(), + plugin.getPluginVersion(), + String.join("; ", unresolvedDependencies) + ); + } + /** * 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 1f231066601..893beb5ed7b 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 @@ -372,6 +372,53 @@ class RepeatedProbeGrailsPlugin { cleanup: System.setErr(originalErr) } + + def 'reports every missing or incompatible plugin dependency at WARN'() { + given: 'a discovery bean with one available and one unresolved plugin' + def gcl = new GroovyClassLoader() + def availableDependencyClass = gcl.parseClass(''' +class AvailableDependencyGrailsPlugin { + def version = "1.0.0" +} +''') + def unresolvedDependenciesClass = gcl.parseClass(''' +class UnresolvedDependenciesGrailsPlugin { + def version = "4.0.0" + def dependsOn = [missingDependency: "1.0.0", availableDependency: "2.0.0"] +} +''') + def discovery = new DefaultPluginDiscovery( + [availableDependencyClass, unresolvedDependenciesClass] 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: 'the public discovery API initializes the plugin set' + discovery.init(new StandardEnvironment()) + + then: 'the unresolved plugin remains failed and is not loaded' + discovery.hasFailedPlugin('unresolvedDependencies') + !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: 'normal verbosity does not include a stack trace' + !captured.toString().contains('at org.apache.grails.core.plugins.DefaultPluginDiscovery') + + cleanup: + System.setErr(originalErr) + } } // Test fixture plugin classes for GrailsPluginDiscoverySpec From 9e732621461bec6e48ecd93efb17cd3186f6b6eb Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Mon, 27 Jul 2026 12:44:18 -0500 Subject: [PATCH 2/2] 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 --- .../core/plugins/DefaultPluginDiscovery.java | 39 ++++++++++-- .../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 c6d33d86894..f3f25731dce 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 @@ private void logUnresolvedDependencies(PluginInfo plugin) { 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 @@ private void logUnresolvedDependencies(PluginInfo plugin) { ); } + /** + * 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 893beb5ed7b..a809720e0c1 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