Improve unresolved plugin dependency diagnostics by jamesfredley · Pull Request #15995 · apache/grails-core

jamesfredley · 2026-07-17T17:52:18Z

Summary

replace the malformed single-line ERROR for an unresolved plugin with one concise WARN
enumerate each missing or version-incompatible dependency with required and actual versions
reuse existing resolution rules; no change to discovery, registration, failed-plugin tracking, or load order

Verification

:grails-core:test --tests PluginDiscoverySpec
:grails-core:test
git diff --check

This is an AI-generated starting point for richer plugin dependency diagnostics.

Assisted-by: opencode:gpt-5.6-sol

Copilot

Pull request overview

This PR improves Grails plugin discovery diagnostics by replacing a malformed unresolved-dependency ERROR with a single WARN that enumerates which plugin dependencies are missing or version-incompatible, without changing the underlying plugin discovery/registration rules.

Changes:

Replace the previous unresolved-plugin ERROR log with a WARN emitted when delayed plugins ultimately fail to resolve.
Add detailed per-dependency messages indicating “missing” vs “version mismatch” (required vs actual).
Add a Spock spec asserting the new WARN content and that no stack trace is printed at normal verbosity.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
grails-core/src/main/groovy/org/apache/grails/core/plugins/DefaultPluginDiscovery.java	Introduces `logUnresolvedDependencies()` to emit a single detailed WARN for plugins that fail dependency resolution.
grails-core/src/test/groovy/org/apache/grails/core/plugins/PluginDiscoverySpec.groovy	Adds a test to validate the new unresolved dependency WARN format and content.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    private void logUnresolvedDependencies(PluginInfo plugin) {
+        var unresolvedDependencies = new ArrayList<String>();
+        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)
+        );
+    }


bito-code-review · 2026-07-17T17:56:17Z

The suggestion to improve logUnresolvedDependencies() by checking delayedLoadPlugins and failedPlugins is valid and would enhance the accuracy of the dependency resolution warnings. Currently, the method only uses findPlugin(name), which returns null if a plugin is not registered, causing it to be incorrectly classified as "missing" even if it exists but failed to load or is delayed.

To implement this, you can update the logic to check these collections before defaulting to the "missing" classification:

private void logUnresolvedDependencies(PluginInfo plugin) {
    var unresolvedDependencies = new ArrayList<String>();
    for (var name : plugin.getDependsOnNames()) {
        var requiredVersion = plugin.getMetadata().getDependentVersion(name);
        var dependency = findPlugin(name);
        
        if (dependency == null) {
            // Check if it exists in failed or delayed collections
            if (failedPlugins.containsKey(name)) {
                unresolvedDependencies.add("dependency [" + name + "] failed to load");
            } else if (delayedLoadPlugins.stream().anyMatch(p -> p.getName().equals(name))) {
                unresolvedDependencies.add("dependency [" + name + "] is delayed");
            } else {
                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 warning
}

grails-core/src/main/groovy/org/apache/grails/core/plugins/DefaultPluginDiscovery.java

if (dependency == null) {
            // Check if it exists in failed or delayed collections
            if (failedPlugins.containsKey(name)) {
                unresolvedDependencies.add("dependency [" + name + "] failed to load");
            } else if (delayedLoadPlugins.stream().anyMatch(p -> p.getName().equals(name))) {
                unresolvedDependencies.add("dependency [" + name + "] is delayed");
            } else {
                unresolvedDependencies.add("dependency [" + name + "] with required version [" + requiredVersion + "] is missing");
            }
        } else if (!GrailsVersionUtils.isValidVersion(dependency.getPluginVersion(), requiredVersion)) {

codecov · 2026-07-17T18:31:26Z

Codecov Report

❌ Patch coverage is 85.18519% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.4999%. Comparing base (b980413) to head (9e73262).
⚠️ Report is 169 commits behind head on 8.0.x.

Files with missing lines	Patch %	Lines
...he/grails/core/plugins/DefaultPluginDiscovery.java	85.1852%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@               Coverage Diff                @@
##             8.0.x     #15995         +/-   ##
================================================
+ Coverage         0   51.4999%   +51.4999%     
- Complexity       0      17791      +17791     
================================================
  Files            0       2039       +2039     
  Lines            0      95608      +95608     
  Branches         0      16598      +16598     
================================================
+ Hits             0      49238      +49238     
- Misses           0      39059      +39059     
- Partials         0       7311       +7311

Files with missing lines	Coverage Δ
...he/grails/core/plugins/DefaultPluginDiscovery.java	`60.8996% <85.1852%> (ø)`

... and 2038 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

davydotcom

Looks good — approving.

Failure determination is unchanged: plugins still fail the same way via the existing dependency resolution / delayed-load path into failedPlugins. Discovery, registration, failed-plugin tracking, and load order are untouched.

What’s new here is the notification: replacing the malformed ERROR with a single concise WARN that enumerates missing vs version-incompatible dependencies. That’s a clearer alert for users without changing how unresolved plugins are decided.

jdaugherty · 2026-07-17T20:41:53Z

+                );
+            }
+        }
+        LOG.warn(


I disagree on this being a warning, there is a legitimate configuration issue. This should be an error message

davydotcom

Revising my earlier approval with one requested change.

The richer diagnostics look good, and failure determination is still unchanged — this is still just a clearer alert for an already-failed plugin. Please keep the log level at ERROR, not WARN.

The previous message was already LOG.error. Downgrading to WARN isn’t justified in the PR beyond “one concise WARN,” and unresolved/failed plugin dependencies are a real load failure. ERROR keeps the severity aligned with that outcome and with prior behavior; WARN can be filtered out of environments that only surface errors.

Requested change:

use LOG.error(...) in logUnresolvedDependencies
update the new PluginDiscoverySpec assertion accordingly (ERROR instead of WARN)

davydotcom · 2026-07-17T20:42:42Z

+                );
+            }
+        }
+        LOG.warn(


Please keep this at ERROR rather than WARN.

Unresolved dependencies already put the plugin in failedPlugins; that is a load failure, and the previous code logged it with LOG.error. A clearer message is welcome, but the severity should stay ERROR so it isn’t lost when WARN is filtered.

- 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 <noreply@anthropic.com>

testlens-app · 2026-07-27T20:57:53Z

✅ All tests passed ✅

🏷️ Commit: 9e73262
▶️ Tests: 38708 executed
⚪️ Checks: 59/59 completed

Learn more about TestLens at testlens.app.

Improve unresolved plugin dependency diagnostics

9e30fb0

Assisted-by: opencode:gpt-5.6-sol

Copilot AI review requested due to automatic review settings July 17, 2026 17:52

jamesfredley added the ai-generated-starting-point label Jul 17, 2026

github-project-automation Bot added this to Apache Grails Jul 17, 2026

Copilot started reviewing on behalf of jamesfredley July 17, 2026 17:52 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

davydotcom approved these changes Jul 17, 2026

View reviewed changes

jdaugherty requested changes Jul 17, 2026

View reviewed changes

davydotcom requested changes Jul 17, 2026

View reviewed changes

davydotcom reviewed Jul 17, 2026

View reviewed changes

jamesfredley moved this to Todo in Apache Grails Jul 24, 2026

borinquenkid added this to the grails:8.0.0-RC1 milestone Jul 27, 2026

jamesfredley modified the milestones: grails:8.0.0-RC1, grails:8.0.0-RC2 Aug 6, 2026

Navigation Menu

Uh oh!

Uh oh!

Improve unresolved plugin dependency diagnostics - #15995

Improve unresolved plugin dependency diagnostics#15995
jamesfredley wants to merge 2 commits into
8.0.xfrom
fix/plugin-dependency-diagnostics

jamesfredley commented Jul 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

bito-code-review Bot commented Jul 17, 2026

Uh oh!

codecov Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

davydotcom left a comment

Uh oh!

jdaugherty Jul 17, 2026

Uh oh!

davydotcom left a comment

Uh oh!

davydotcom Jul 17, 2026

Uh oh!

testlens-app Bot commented Jul 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Uh oh!

Conversation

jamesfredley commented Jul 17, 2026

Summary

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

bito-code-review Bot commented Jul 17, 2026

Uh oh!

codecov Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

davydotcom left a comment

Choose a reason for hiding this comment

Uh oh!

jdaugherty Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

davydotcom left a comment

Choose a reason for hiding this comment

Uh oh!

davydotcom Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

testlens-app Bot commented Jul 27, 2026

✅ All tests passed ✅

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Jul 17, 2026 •

edited

Loading