You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jamesfredley (via GitHub)" <gi...@apache.org> on 2026/07/17 17:52:21 UTC
[PR] Improve unresolved plugin dependency diagnostics [grails-core]
jamesfredley opened a new pull request, #15995:
URL: https://github.com/apache/grails-core/pull/15995
## 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.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve unresolved plugin dependency diagnostics [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #15995:
URL: https://github.com/apache/grails-core/pull/15995#issuecomment-5804690813
The original issue has been addressed, so merging.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve unresolved plugin dependency diagnostics [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #15995:
URL: https://github.com/apache/grails-core/pull/15995#issuecomment-5804670356
Reviewed the branch against current `8.0.x` and pushed two follow-up commits (a merge of `8.0.x` plus a fix) so this can go in.
**What was wrong**
- The log level is already back at ERROR from the earlier feedback, and that stays. Both new spec features failed once merged with `8.0.x`, though: they captured `System.err` expecting slf4j-simple output, but `DefaultPluginDiscovery` logging now goes through logback on the base branch, so the capture saw nothing (`errors.size() == 1` failed with `[]`, and the second feature hit an NPE on a null line).
- The `is still pending load` branch hid the real cause. A plugin only fails while a same-named dependency is still delayed when that delayed candidate does not satisfy the version constraint (`isDependentOn` checks name and version before re-queuing). So the message reported the dependency as merely pending, when the actual problem was the version.
**What changed**
- Rewrote both features on the existing `LogCapture` test fixture and assert on `Level.ERROR` plus the formatted message, matching the neighbouring duplicate-registration test.
- `isDelayed(name)` became `findDelayedPlugin(name)`, and the message now reads `dependency [x] with required version [2.0.0] is still pending load and only version [1.0.0] was found`. Added a feature that drives that path (dependent processed before its too-old, still-delayed dependency gives up).
**Verification**
- `:grails-core:test` full module: 657 tests, 0 failures
- `:grails-core:codeStyle` clean
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve unresolved plugin dependency diagnostics [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty merged PR #15995:
URL: https://github.com/apache/grails-core/pull/15995
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve unresolved plugin dependency diagnostics [grails-core]
Posted by "davydotcom (via GitHub)" <gi...@apache.org>.
davydotcom commented on code in PR #15995:
URL: https://github.com/apache/grails-core/pull/15995#discussion_r3606024258
##########
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<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(
Review Comment:
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.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve unresolved plugin dependency diagnostics [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15995:
URL: https://github.com/apache/grails-core/pull/15995#issuecomment-5006907313
## ✅ All tests passed ✅
🏷️ Commit: 9e30fb08e5fa64ba838259bfff1e69bd2c10cc25
▶️ Tests: 27809 executed
⚪️ Checks: 61/61 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve unresolved plugin dependency diagnostics [grails-core]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #15995:
URL: https://github.com/apache/grails-core/pull/15995#discussion_r3605145430
##########
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<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)
+ );
+ }
Review Comment:
logUnresolvedDependencies() classifies a dependency as "missing" whenever it isn't already registered (findPlugin returns null). That can be misleading when the dependency plugin is present but ended up in delayedLoadPlugins or failedPlugins (e.g., it exists but failed to load due to its own unresolved deps), and it can also lead to an empty reason list if the only unresolved deps are failed-but-version-compatible. Consider also checking failedPlugins/delayedLoadPlugins by name so the WARN accurately distinguishes missing vs failed vs version-incompatible dependencies.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve unresolved plugin dependency diagnostics [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #15995:
URL: https://github.com/apache/grails-core/pull/15995#issuecomment-5006347635
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/15995?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 51.1209%. Comparing base ([`b980413`](https://app.codecov.io/gh/apache/grails-core/commit/b980413820757347744aa6c10c44d7103babea39?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`9e30fb0`](https://app.codecov.io/gh/apache/grails-core/commit/9e30fb08e5fa64ba838259bfff1e69bd2c10cc25?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/15995?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ 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](https://app.codecov.io/gh/apache/grails-core/pull/15995?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...he/grails/core/plugins/DefaultPluginDiscovery.java](https://app.codecov.io/gh/apache/grails-core/pull/15995?src=pr&el=tree&filepath=grails-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fapache%2Fgrails%2Fcore%2Fplugins%2FDefaultPluginDiscovery.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9hcGFjaGUvZ3JhaWxzL2NvcmUvcGx1Z2lucy9EZWZhdWx0UGx1Z2luRGlzY292ZXJ5LmphdmE=) | `60.5735% <100.0000%> (ø)` | |
... and [2040 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/15995/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve unresolved plugin dependency diagnostics [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15995:
URL: https://github.com/apache/grails-core/pull/15995#issuecomment-5096746710
## ✅ All tests passed ✅
🏷️ Commit: 9e732621461bec6e48ecd93efb17cd3186f6b6eb
▶️ Tests: 38708 executed
⚪️ Checks: 59/59 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve unresolved plugin dependency diagnostics [grails-core]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #15995:
URL: https://github.com/apache/grails-core/pull/15995#issuecomment-5006002306
<!-- Bito Reply -->
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:
```groovy
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)) {
```
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve unresolved plugin dependency diagnostics [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15995:
URL: https://github.com/apache/grails-core/pull/15995#discussion_r3606020475
##########
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<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(
Review Comment:
I disagree on this being a warning, there is a legitimate configuration issue. This should be an error message
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org