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

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.1209%. Comparing base (b980413) to head (9e30fb0).

Additional details and impacted files

@@               Coverage Diff                @@
##             8.0.x     #15995         +/-   ##
================================================
+ Coverage         0   51.1209%   +51.1209%     
- Complexity       0      17604      +17604     
================================================
  Files            0       2041       +2041     
  Lines            0      95505      +95505     
  Branches         0      16590      +16590     
================================================
+ Hits             0      48823      +48823     
- Misses           0      39397      +39397     
- Partials         0       7285       +7285

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

... and 2040 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.

testlens-app · 2026-07-17T19:45:12Z

✅ All tests passed ✅

🏷️ Commit: 9e30fb0
▶️ Tests: 27809 executed
⚪️ Checks: 61/61 completed

Learn more about TestLens at testlens.app.

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.

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Improve unresolved plugin dependency diagnostics#15995

Improve unresolved plugin dependency diagnostics#15995
jamesfredley wants to merge 1 commit 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

Uh oh!

testlens-app Bot commented Jul 17, 2026

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 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

Codecov Report

Uh oh!

testlens-app Bot commented Jul 17, 2026

✅ All tests passed ✅

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants