[
https://issues.apache.org/jira/browse/GROOVY-12683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18137612#comment-18137612
]
ASF GitHub Bot commented on GROOVY-12683:
-----------------------------------------
codecov-commenter commented on PR #3204:
URL: https://github.com/apache/groovy/pull/3204#issuecomment-6617186670
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/3204?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:x: Patch coverage is `52.52525%` with `47 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 74.1402%. Comparing base
([`0941dba`](https://app.codecov.io/gh/apache/groovy/commit/0941dbacf496a1667c27681c6fe76deee59bfb57?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`7b341cf`](https://app.codecov.io/gh/apache/groovy/commit/7b341cf15b1296a79e87d58eb75c5d36f5d8da98?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 12 commits behind head on master.
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3204?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...us/groovy/runtime/m12n/ExtensionModuleScanner.java](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2Fm12n%2FExtensionModuleScanner.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvbTEybi9FeHRlbnNpb25Nb2R1bGVTY2FubmVyLmphdmE=)
| 41.2698% | [29 Missing and 8 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[src/main/groovy/groovy/grape/GrapeUtil.groovy](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&filepath=src%2Fmain%2Fgroovy%2Fgroovy%2Fgrape%2FGrapeUtil.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vZ3Jvb3Z5L2dyb292eS9ncmFwZS9HcmFwZVV0aWwuZ3Jvb3Z5)
| 68.4210% | [3 Missing and 3 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...vy/transform/stc/AbstractExtensionMethodCache.java](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2Fstc%2FAbstractExtensionMethodCache.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9zdGMvQWJzdHJhY3RFeHRlbnNpb25NZXRob2RDYWNoZS5qYXZh)
| 80.0000% | [3 Missing :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...c/main/groovy/groovy/grape/maven/GrapeMaven.groovy](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&filepath=subprojects%2Fgroovy-grape-maven%2Fsrc%2Fmain%2Fgroovy%2Fgroovy%2Fgrape%2Fmaven%2FGrapeMaven.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWdyYXBlLW1hdmVuL3NyYy9tYWluL2dyb292eS9ncm9vdnkvZ3JhcGUvbWF2ZW4vR3JhcGVNYXZlbi5ncm9vdnk=)
| 0.0000% | [1 Missing :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&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/groovy/pull/3204?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #3204 +/- ##
==================================================
- Coverage 74.1533% 74.1402% -0.0131%
- Complexity 43751 43765 +14
==================================================
Files 1692 1692
Lines 149516 149603 +87
Branches 28366 28381 +15
==================================================
+ Hits 110871 110916 +45
- Misses 28448 28481 +33
- Partials 10197 10206 +9
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3204?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...us/groovy/runtime/m12n/MetaInfExtensionModule.java](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2Fm12n%2FMetaInfExtensionModule.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvbTEybi9NZXRhSW5mRXh0ZW5zaW9uTW9kdWxlLmphdmE=)
| `73.0769% <ø> (ø)` | |
|
[...y/src/main/groovy/groovy/grape/ivy/GrapeIvy.groovy](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&filepath=subprojects%2Fgroovy-grape-ivy%2Fsrc%2Fmain%2Fgroovy%2Fgroovy%2Fgrape%2Fivy%2FGrapeIvy.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWdyYXBlLWl2eS9zcmMvbWFpbi9ncm9vdnkvZ3Jvb3Z5L2dyYXBlL2l2eS9HcmFwZUl2eS5ncm9vdnk=)
| `33.9066% <100.0000%> (+0.1628%)` | :arrow_up: |
|
[...c/main/groovy/groovy/grape/maven/GrapeMaven.groovy](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&filepath=subprojects%2Fgroovy-grape-maven%2Fsrc%2Fmain%2Fgroovy%2Fgroovy%2Fgrape%2Fmaven%2FGrapeMaven.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWdyYXBlLW1hdmVuL3NyYy9tYWluL2dyb292eS9ncm9vdnkvZ3JhcGUvbWF2ZW4vR3JhcGVNYXZlbi5ncm9vdnk=)
| `0.0000% <0.0000%> (ø)` | |
|
[...vy/transform/stc/AbstractExtensionMethodCache.java](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2Fstc%2FAbstractExtensionMethodCache.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9zdGMvQWJzdHJhY3RFeHRlbnNpb25NZXRob2RDYWNoZS5qYXZh)
| `81.7460% <80.0000%> (-0.7101%)` | :arrow_down: |
|
[src/main/groovy/groovy/grape/GrapeUtil.groovy](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&filepath=src%2Fmain%2Fgroovy%2Fgroovy%2Fgrape%2FGrapeUtil.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vZ3Jvb3Z5L2dyb292eS9ncmFwZS9HcmFwZVV0aWwuZ3Jvb3Z5)
| `55.0000% <68.4210%> (+1.0000%)` | :arrow_up: |
|
[...us/groovy/runtime/m12n/ExtensionModuleScanner.java](https://app.codecov.io/gh/apache/groovy/pull/3204?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2Fm12n%2FExtensionModuleScanner.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvbTEybi9FeHRlbnNpb25Nb2R1bGVTY2FubmVyLmphdmE=)
| `52.1739% <41.2698%> (-23.6882%)` | :arrow_down: |
... and [11 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/3204/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>
> Extension modules whose classes need an optional dependency break all
> compilation when that dependency is missing (groovy-fx in the distribution)
> -------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12683
> URL: https://issues.apache.org/jira/browse/GROOVY-12683
> Project: Groovy
> Issue Type: Bug
> Components: Extension methods, Grape
> Reporter: Paul King
> Priority: Major
>
> h3. Problem
> The {{groovy-fx}} module added by GROOVY-11434 is an optional module, but it
> is still copied into {{lib}} of the binary distribution and the
> {{installGroovy}} task output. JavaFX is a compile-only dependency of that
> module and is not bundled. On a JDK without JavaFX, that one jar makes
> *every* compilation fail, even a trivial script:
> {noformat}
> $ groovy -e 'println "x".toUpperCase()'
> SEVERE [groovy.ui.GroovyMain] Caught: {0}
> java.lang.NoClassDefFoundError: Unable to configure
> org.apache.groovy.groovyfx.extensions.FXExtensions due to missing dependency
> javafx/beans/binding/NumberBinding
> {noformat}
> Removing {{groovy-fx-*.jar}} from {{lib}} makes the error go away.
> h3. Cause
> The module descriptor registers {{FXExtensions}}, whose method signatures
> mention JavaFX types. The runtime already tolerates an extension class it
> cannot link: {{SimpleExtensionModule.getMetaMethods}} catches
> {{LinkageError}} per class and logs a warning. The compile-time cache does
> not. {{AbstractExtensionMethodCache.scan}}, which backs both static type
> checking and the macro transform, builds a {{ClassNode}} for every extension
> class visible to the loader. When one class cannot be configured, the error
> escapes and stops the compilation. The macro transform is a global transform,
> so this happens for every script, not just ones that use {{@CompileStatic}}.
> GroovyFX is the first module we ship whose descriptor-registered extension
> class depends on a compile-only dependency. A third-party extension module
> with an optional dependency fails in exactly the same way.
> h3. Proposed fix
> # *Backstop:* make {{AbstractExtensionMethodCache.scan}} catch
> {{LinkageError}} per extension class and log a warning that names the class,
> matching the runtime. A single unusable module can then no longer stop all
> compilation.
> # *Declared prerequisites:* add an optional {{requiredClasses}} key to the
> extension module descriptor. If any listed class cannot be loaded from the
> loader that defines the module's extension classes, the module is deferred
> instead of loaded. It is not registered, a FINE message reports the deferral,
> and its descriptor is remembered per class loader.
> # *Late activation:* after each {{@Grab}}, Grape (Ivy and Maven) re-checks
> the deferred modules. It registers any whose required classes are now
> loadable, using the existing GROOVY-5543 metaclass refresh. It then
> invalidates the compile-time extension method caches so static type checking
> and macros see the new methods. Grape does not invalidate those caches today,
> even for an ordinary grabbed extension module.
> # groovy-fx declares {{requiredClasses=javafx.beans.binding.NumberBinding}}
> through the build-logic {{moduleDescriptor}} DSL.
> Users who supply JavaFX through a build file, a JavaFX-bundling JDK or
> {{--module-path}} are unaffected: the probe succeeds at startup and the
> module registers as before.
> h3. Class loader caveat
> A jar in {{lib}} is defined by the launcher's {{RootLoader}}, so its classes
> can only link against classes that loader can see. A plain {{@Grab}} adds
> JavaFX to the script's child {{GroovyClassLoader}}, which the module cannot
> use. In that case the module stays deferred and a warning suggests
> {{@GrabConfig(systemClassLoader=true)}}, which works end to end. The same
> rule already applies to {{SceneGraphBuilder}} and every other class in the
> jar. The GroovyFX Grape recipe in the user guide should use
> {{systemClassLoader=true}}.
> h3. Verified with a prototype
> * A new forked-JVM test, {{DeferredExtensionModuleTest}}, covers four cases.
> A module is deferred, then activates for dynamic and {{@CompileStatic}} code.
> A module with its required class present loads normally. A module stays
> deferred when the class is visible only to a child loader. A module with an
> unlinkable signature no longer breaks compilation. That last case fails on
> master with the error above.
> * On an installed build that has groovy-fx in {{lib}} and no JavaFX, plain
> and {{@CompileStatic}} scripts run.
> * With {{@GrabConfig(systemClassLoader=true)}} and {{@Grab}} of
> {{javafx-base}}, the module activates, and {{SimpleIntegerProperty +
> SimpleIntegerProperty}} works both dynamically and under {{@CompileStatic}}.
> * The existing extension-module, {{DgmMethodLookupTest}}, Grape, groovy-macro
> and groovy-fx tests still pass.
> h3. Notes
> * The change adds new public API, all {{@since 7.0.0}}: the descriptor key
> constant, two static methods on {{ExtensionModuleScanner}}, and
> {{AbstractExtensionMethodCache.invalidateAllCaches()}}. The scanner methods
> could be made internal instead.
> * The descriptor key is additive. Older Groovy versions ignore it.
> * A simpler stopgap for 7.0.0-alpha-1 is to exclude groovy-fx from the
> distribution's {{lib}}. That stops the breakage without the new mechanism.
> * Separate cosmetic bug seen above: {{GroovyMain}} logs {{"Caught: {0}"}}
> without formatting, so the literal {{{0}}} is shown.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
