Package libraries Java code as Java Modules #10714
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JaroslavTulach
requested review from
jdunkerley,
radeusgd,
GregoryTravis,
AdRiley,
marthasharkey,
4e6,
hubertp and
Akirathan
as code owners
JaroslavTulach
force-pushed
the
wip/jtulach/ClassPath7082
branch
from
ba4db96 to
873c418
Compare
JaroslavTulach
commented
Jul 30, 2024
engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java
Outdated
Show resolved
Hide resolved
|
Question for @radeusgd - we seem to get the encapsulation with all the consequences. There is dozen of failures:
I can think of ways to fix the tests, but some failures show intended cross-library usage we don't have any alternative yet. |
I imagine we can come up with some workarounds, although that may not be trivial. I'm worried the most about the read autodetection - does it look like the classpath separation breaks the SPI? How can we fix that? Alternatively, as suggested in #7082 (title Re-exporting), could we provide a way for a library to opt-in to being able to access modules of another lib? e.g. |
JaroslavTulach
added
the
CI: Clean build required
JaroslavTulach
commented
Jul 30, 2024
My plan was: enso$ git diff distribution/ test/Base_Tests/src/Data/Text/Utils_Spec.enso
diff --git distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Extra_Imports.enso distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Extra_Imports.enso
index 7771401cf4..c42ee36ec6 100644
--- distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Extra_Imports.enso
+++ distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Extra_Imports.enso
@@ -9,7 +9,7 @@ private
polyglot java import java.io.PrintStream
# needed by Util_Spec
-polyglot java import org.enso.base.text.CaseFoldedString
+polyglot java import org.enso.base.text.CaseFoldedString as JCaseFoldedString
polyglot java import org.enso.base.text.CaseFoldedString.Grapheme
# needed by Comparator_Spec:
@@ -29,3 +29,5 @@ polyglot java import java.util.function.Function
polyglot java import java.lang.Thread
polyglot java import java.lang.Thread.State
polyglot java import java.lang.Float
+
+CaseFoldedString = JCaseFoldedString
diff --git test/Base_Tests/src/Data/Text/Utils_Spec.enso test/Base_Tests/src/Data/Text/Utils_Spec.enso
index d0b987bea2..7d8d006ff7 100644
--- test/Base_Tests/src/Data/Text/Utils_Spec.enso
+++ test/Base_Tests/src/Data/Text/Utils_Spec.enso
@@ -1,7 +1,7 @@
from Standard.Base import all
polyglot java import org.enso.base.Text_Utils
-polyglot java import org.enso.base.text.CaseFoldedString
+import Standard.Base.Internal.Extra_Imports.CaseFoldedString
polyglot java import com.ibm.icu.text.BreakIterator
from Standard.Test import allbut that requires us to run with I guess we cannot have both: Run with |
|
Heuréka. JVM tests are green. |
…et.http.UrlencodedBodyBuilder UnresolvedSymbol<new>)
JaroslavTulach
commented
Oct 10, 2024
4 tasks
|
There are seven
I need to find a way to simulate the problem locally! Update: I manage to simulate similar problem with 208d05d: looks like there is a problem in discovery of |
This |
2 tasks
5 tasks
|
I tried to workaround the GraalVM diff --git build.sbt build.sbt
index 101f03aa16..f3d1e4a9c9 100644
--- build.sbt
+++ build.sbt
@@ -3777,7 +3777,6 @@ lazy val `engine-runner` = project
"com.sun.jna",
"com.microsoft",
"akka.http",
- "org.enso.base",
"org.enso.image",
"org.enso.table"
)
diff --git engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java
index 433271da46..2d17c38fcb 100644
--- engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java
+++ engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java
@@ -16,6 +16,34 @@ import org.graalvm.nativeimage.hosted.RuntimeReflection;
public final class EnsoLibraryFeature implements Feature {
@Override
public void beforeAnalysis(BeforeAnalysisAccess access) {
+ try {
+ var lookupClass = access.findClassByName("org.enso.base.lookup.Lookup");
+ var foundField = lookupClass.getDeclaredField("found");
+ foundField.setAccessible(true);
+ var reloadMethod = lookupClass.getDeclaredMethod("reload");
+ access.registerFieldValueTransformer(
+ foundField,
+ (receiver, originalValue) -> {
+ try {
+ System.err.println(" transform " + receiver);
+ reloadMethod.invoke(receiver);
+ var newValue = foundField.get(receiver);
+ System.err.println(" value " + newValue);
+ return newValue;
+ } catch (ReflectiveOperationException ex) {
+ throw new IllegalStateException(ex);
+ }
+ });
+
+ } catch (ReflectiveOperationException | SecurityException ex) {
+ throw new IllegalStateException(ex);
+ }
+
+ registerPolyglotJavaImportForReflectiveAccess(access);
+ }
+
+ private void registerPolyglotJavaImportForReflectiveAccess(BeforeAnalysisAccess access)
+ throws IllegalStateException {
var libs = new LinkedHashSet<Path>();
for (var p : access.getApplicationClassPath()) {
var p1 = p.getParent();
@@ -30,18 +58,18 @@ public final class EnsoLibraryFeature implements Feature {
}
/*
- To run Standard.Test one shall analyze its polyglot/java files. But there are none
- to include on classpath as necessary test classes are included in Standard.Base!
- We can locate the Test library by following code or we can make sure all necessary
- imports are already mentioned in Standard.Base itself.
+ To run Standard.Test one shall analyze its polyglot/java files. But there are none
+ to include on classpath as necessary test classes are included in Standard.Base!
+ We can locate the Test library by following code or we can make sure all necessary
+ imports are already mentioned in Standard.Base itself.
if (!libs.isEmpty()) {
- var f = libs.iterator().next();
- var stdTest = f.getParent().getParent().resolve("Test").resolve(f.getFileName());
- if (stdTest.toFile().exists()) {
- libs.add(stdTest);
- }
- System.err.println("Testing library: " + stdTest);
+ var f = libs.iterator().next();
+ var stdTest = f.getParent().getParent().resolve("Test").resolve(f.getFileName());
+ if (stdTest.toFile().exists()) {
+ libs.add(stdTest);
+ }
+ System.err.println("Testing library: " + stdTest);
}
*/
diff --git engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoClassPath.java engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoClassPath.java
index 2d249e3401..bf9a7f11ea 100644
--- engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoClassPath.java
+++ engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoClassPath.java
@@ -120,6 +120,7 @@ public final class EnsoClassPath {
@SuppressWarnings("unchecked")
private static void registerLayer(ModuleLayer moduleLayer) {
+ new Exception("registerLayer").printStackTrace();
var props = System.getProperties();
Collection<ModuleLayer> layers;
if (props.get("enso.class.path") instanceof Collection registeredLayers) {
diff --git std-bits/base/src/main/java/org/enso/base/lookup/Lookup.java std-bits/base/src/main/java/org/enso/base/lookup/Lookup.java
index 439bf03f22..f42b182d27 100644
--- std-bits/base/src/main/java/org/enso/base/lookup/Lookup.java
+++ std-bits/base/src/main/java/org/enso/base/lookup/Lookup.java
@@ -33,7 +33,7 @@ public final class Lookup<T> implements Iterable<T> {
}
public void reload() {
- found = null;
+ found = findAll();
}
private List<ServiceLoader.Provider<T>> findAll() {The idea is to populate all the |
2 tasks
5 tasks
|
This PR oracle/graal#10202 shows the substitutions needed to make the
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Implements #7082 by creating a module layer per library. Removes the obsolete add_to_class_path concept as there is no longer a single
ClassPathto load from. Each library withpolyglot/javaJARs gets its own. Need a way to express dependencies between Java APIs among multiple libraries - createdrequires:flag inpackage.yaml.Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.