[
https://issues.apache.org/jira/browse/GROOVY-12751?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18141460#comment-18141460
]
ASF GitHub Bot commented on GROOVY-12751:
-----------------------------------------
codecov-commenter commented on PR #3284:
URL: https://github.com/apache/groovy/pull/3284#issuecomment-6746513054
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/3284?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 `89.62963%` with `42 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 74.4689%. Comparing base
([`cb1b208`](https://app.codecov.io/gh/apache/groovy/commit/cb1b208fbbf57eac3cee52eb786eadc9277ec1c2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`c61a950`](https://app.codecov.io/gh/apache/groovy/commit/c61a950f187f93958a15abfc8f66102dc508b2e7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 25 commits behind head on master.
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3284?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/tools/javac/InMemoryJavaFileManager.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftools%2Fjavac%2FInMemoryJavaFileManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3Rvb2xzL2phdmFjL0luTWVtb3J5SmF2YUZpbGVNYW5hZ2VyLmphdmE=)
| 85.4839% | [5 Missing and 13 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...codehaus/groovy/tools/javac/JavacJavaCompiler.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftools%2Fjavac%2FJavacJavaCompiler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3Rvb2xzL2phdmFjL0phdmFjSmF2YUNvbXBpbGVyLmphdmE=)
| 73.3333% | [0 Missing and 8 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...s/groovy/tools/javac/JavaAwareCompilationUnit.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftools%2Fjavac%2FJavaAwareCompilationUnit.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3Rvb2xzL2phdmFjL0phdmFBd2FyZUNvbXBpbGF0aW9uVW5pdC5qYXZh)
| 89.7959% | [2 Missing and 3 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...us/groovy/tools/javac/MemClassOutputClasspath.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftools%2Fjavac%2FMemClassOutputClasspath.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3Rvb2xzL2phdmFjL01lbUNsYXNzT3V0cHV0Q2xhc3NwYXRoLmphdmE=)
| 96.4602% | [2 Missing and 2 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...e/src/main/groovy/groovy/console/ui/Console.groovy](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=subprojects%2Fgroovy-console%2Fsrc%2Fmain%2Fgroovy%2Fgroovy%2Fconsole%2Fui%2FConsole.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWNvbnNvbGUvc3JjL21haW4vZ3Jvb3Z5L2dyb292eS9jb25zb2xlL3VpL0NvbnNvbGUuZ3Jvb3Z5)
| 0.0000% | [4 Missing :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...rg/codehaus/groovy/control/ModuleImportHelper.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2FModuleImportHelper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvTW9kdWxlSW1wb3J0SGVscGVyLmphdmE=)
| 33.3333% | [1 Missing and 1 partial :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...rg/apache/groovy/groovysh/jline/PackageHelper.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=subprojects%2Fgroovy-groovysh%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fapache%2Fgroovy%2Fgroovysh%2Fjline%2FPackageHelper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWdyb292eXNoL3NyYy9tYWluL2dyb292eS9vcmcvYXBhY2hlL2dyb292eS9ncm9vdnlzaC9qbGluZS9QYWNrYWdlSGVscGVyLmphdmE=)
| 0.0000% | [1 Missing :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3284?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/3284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #3284 +/- ##
==================================================
+ Coverage 74.4126% 74.4689% +0.0563%
- Complexity 44372 44879 +507
==================================================
Files 1699 1705 +6
Lines 151051 152230 +1179
Branches 28783 29165 +382
==================================================
+ Hits 112401 113364 +963
- Misses 28376 28434 +58
- Partials 10274 10432 +158
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3284?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[.../codehaus/groovy/ast/decompiled/AsmDecompiler.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fast%2Fdecompiled%2FAsmDecompiler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2FzdC9kZWNvbXBpbGVkL0FzbURlY29tcGlsZXIuamF2YQ==)
| `96.7391% <100.0000%> (+0.2275%)` | :arrow_up: |
|
[...a/org/codehaus/groovy/control/CompilationUnit.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2FCompilationUnit.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvQ29tcGlsYXRpb25Vbml0LmphdmE=)
| `81.5939% <100.0000%> (+0.0701%)` | :arrow_up: |
|
[...codehaus/groovy/control/CompilerConfiguration.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2FCompilerConfiguration.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvQ29tcGlsZXJDb25maWd1cmF0aW9uLmphdmE=)
| `75.4821% <100.0000%> (+0.0631%)` | :arrow_up: |
|
[...odehaus/groovy/tools/javac/MemClassFileObject.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftools%2Fjavac%2FMemClassFileObject.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3Rvb2xzL2phdmFjL01lbUNsYXNzRmlsZU9iamVjdC5qYXZh)
| `100.0000% <100.0000%> (ø)` | |
|
[.../groovy/tools/javac/MemClassOutputPublication.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftools%2Fjavac%2FMemClassOutputPublication.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3Rvb2xzL2phdmFjL01lbUNsYXNzT3V0cHV0UHVibGljYXRpb24uamF2YQ==)
| `100.0000% <100.0000%> (ø)` | |
|
[...rg/apache/groovy/groovysh/jline/PackageHelper.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=subprojects%2Fgroovy-groovysh%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fapache%2Fgroovy%2Fgroovysh%2Fjline%2FPackageHelper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWdyb292eXNoL3NyYy9tYWluL2dyb292eS9vcmcvYXBhY2hlL2dyb292eS9ncm9vdnlzaC9qbGluZS9QYWNrYWdlSGVscGVyLmphdmE=)
| `45.2174% <0.0000%> (+0.3898%)` | :arrow_up: |
|
[...rg/codehaus/groovy/control/ModuleImportHelper.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2FModuleImportHelper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvTW9kdWxlSW1wb3J0SGVscGVyLmphdmE=)
| `73.8095% <33.3333%> (-1.1905%)` | :arrow_down: |
|
[...us/groovy/tools/javac/MemClassOutputClasspath.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftools%2Fjavac%2FMemClassOutputClasspath.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3Rvb2xzL2phdmFjL01lbUNsYXNzT3V0cHV0Q2xhc3NwYXRoLmphdmE=)
| `96.4602% <96.4602%> (ø)` | |
|
[...e/src/main/groovy/groovy/console/ui/Console.groovy](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=subprojects%2Fgroovy-console%2Fsrc%2Fmain%2Fgroovy%2Fgroovy%2Fconsole%2Fui%2FConsole.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWNvbnNvbGUvc3JjL21haW4vZ3Jvb3Z5L2dyb292eS9jb25zb2xlL3VpL0NvbnNvbGUuZ3Jvb3Z5)
| `28.0335% <0.0000%> (-0.0706%)` | :arrow_down: |
|
[...s/groovy/tools/javac/JavaAwareCompilationUnit.java](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftools%2Fjavac%2FJavaAwareCompilationUnit.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3Rvb2xzL2phdmFjL0phdmFBd2FyZUNvbXBpbGF0aW9uVW5pdC5qYXZh)
| `91.1290% <89.7959%> (-0.9762%)` | :arrow_down: |
| ... and [2
more](https://app.codecov.io/gh/apache/groovy/pull/3284?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| |
... and [47 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/3284/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>
> In-memory javac class output across compilation units (follow-up to
> GROOVY-12711)
> ---------------------------------------------------------------------------------
>
> Key: GROOVY-12751
> URL: https://issues.apache.org/jira/browse/GROOVY-12751
> Project: Groovy
> Issue Type: Improvement
> Components: Compiler
> Reporter: Daniel Sun
> Priority: Major
>
> h3. Background
> GROOVY-12711 ([PR #3242|https://github.com/apache/groovy/pull/3242]) adds the
> joint-compilation option {{memClassOutput}}
> ({{CompilerConfiguration.MEM_CLASS_OUTPUT}}). Together with {{memStub}}, it
> keeps javac's {{CLASS_OUTPUT}} in memory, so a {{JavaAwareCompilationUnit}}
> needs neither a stub directory nor a target directory on disk.
> In that change, the in-memory store is owned by a single compilation unit. An
> earlier revision also kept a map from {{GroovyClassLoader}} to store, so that
> a second compilation unit created on the same loader could see the classes
> published by the first. That map was removed during review: the only scenario
> it served was a test that deliberately built two units on one loader, and
> none of the normal entry points ({{groovyc}}, the Ant task,
> {{GroovyClassLoader#parseClass}}, groovy-lsp) actually reaches it.
> The underlying question is still a fair one, though, and deserves to be
> looked at on its own merits: when a host keeps a long-lived class loader and
> compiles more than once, how should in-memory class output from earlier
> compilations be made visible to later ones?
> h3. Goal
> Define, and if justified implement, a publication rule for in-memory class
> output that spans compilation units, driven by how real hosts use Groovy's
> existing APIs rather than by a hand-built test scenario.
> Concretely, the outcome should answer:
> * Which loader is long-lived, and which loader defines the classes of each
> compilation?
> * Where must the in-memory bytes live so that the loader actually searched by
> {{getResource}} and {{loadClass}} can find them?
> * How do overlapping class names behave when a later compilation redefines a
> class that an earlier one already produced?
> h3. Facts any design has to respect
> * {{URLClassLoader}} cannot remove a URL, and {{getResource}} returns the
> first match in parent-first, insertion order. Appending one more in-memory
> URL per compilation to a long-lived loader therefore lets the *oldest* bytes
> win for overlapping names. That cannot serve as an update mechanism.
> * A class that a loader has already defined cannot be replaced. Recompiling a
> loaded class always requires a new defining loader.
> * {{GroovyClassLoader#parseClass}} already separates the two roles: the outer
> loader is long-lived, while each parse defines its classes on a fresh,
> short-lived {{InnerLoader}} whose {{getResource}} and {{addURL}} delegate to
> the outer loader. Joint compilation does not use this split today. A map
> keyed on the compilation unit's loader does not follow it either.
> * groovy-lsp reuses one loader across compilation units while the compiler
> settings are unchanged, and its recompiles usually overlap the previous ones.
> It sets {{memStub}} but still writes javac output to a temporary directory.
> If it ever adopts {{memClassOutput}}, overlapping names will be the common
> case, not the exception.
> * The Ant task's incremental mode keeps earlier classes visible by putting
> the destination directory on the classpath ({{includeDestClasses}}). An
> in-memory equivalent has to state whether it means parent delegation to
> earlier output, or a fresh compilation of the union of sources.
> h3. Proposed approach
> # Pick one or more concrete hosts (for example an embedder built on
> {{parseClass}}, or groovy-lsp) and describe its loader lifetime through
> existing, public API only.
> # Follow the {{parseClass}} shape: one long-lived parent that holds
> previously published output only when the host asks for it, plus a new child
> loader per compilation that defines that compilation's classes and is
> searched first.
> # Put the in-memory bytes on the loader that the child actually searches,
> rather than appending a sticky URL to a shared loader.
> # Specify what happens for overlapping names and for classes that are already
> defined, and do not suggest that {{loadClass}} on a loader that already
> defined a class will ever see new bytes.
> # Cover it with tests that go through the normal API
> ({{CompilerConfiguration}}, {{JavaAwareCompilationUnit}},
> {{GroovyClassLoader}}), not with two units manually attached to one loader.
> h3. Out of scope
> * Enabling {{memClassOutput}} inside {{parseClass}} or groovy-lsp. Those are
> potential consumers of whatever rule this issue settles, and can be changed
> separately.
> * Any change to the single-unit behaviour delivered by GROOVY-12711.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
