Rewriting org.enso.runner.Main to Java #9810
Conversation
JaroslavTulach
added
the
CI: No changelog needed
|
Bunch of ProjectManagementApiSpec failures. How does one debug them? Do they really run Fixed in 7189b0c |
JaroslavTulach
added
the
CI: Clean build required
|
The current state of the PR is good enough to give us similar options to other GraalVM language launchers, if that's what we want. I'd like to integrate when the CI gets green. |
There was a problem hiding this comment.
Some simplifications could be added, as per comments. But overall this looks OK if we really want to follow this Java path.
I don't like when non-exceptional paths are using exceptions for regular flow (throw exitSuccess()). Makes it hard to grasp how things are being executed.
| private RuntimeException doExit(int exitCode) { | ||
| RunnerLogging.tearDown(); | ||
| System.exit(exitCode); | ||
| return null; |
There was a problem hiding this comment.
That and phantom "throwing" feels like a really weird pattern overall.
There was a problem hiding this comment.
It is necessary to write throw exitFailure() otherwise Java requires a return value or initialization of a local variable defined sooner, but used after the exit statement.
|
|
||
| if (projectMode) { | ||
| var result = PackageManager$.MODULE$.Default().loadPackage(file); | ||
| if (result.isSuccess()) { |
There was a problem hiding this comment.
Feels like just going instanceof woule be easier to read
There was a problem hiding this comment.
It tried that first. It was a mess. However it might be caused by me writing the switch/case statements incorrectly.
| var main = pkg.mainFile(); | ||
| if (!main.exists()) { | ||
| println("Main file does not exist."); | ||
| context.context().close(); |
There was a problem hiding this comment.
Why not put the whole block in try/finally? It's not obvious if the context is closed in every case that follows.
There was a problem hiding this comment.
There is
println("Main file does not exist.");
context.context().close();
throw exitFail();just a few lines above. Try/finally won't help, as exitFail calls System.exit and never returns.
JaroslavTulach
force-pushed
the
wip/jtulach/EnsoLauncher9749
branch
from
f8d44c2
to
ece8746
Compare
JaroslavTulach
changed the title
JaroslavTulach
added
the
CI: Ready to merge
JaroslavTulach
changed the title
Pull Request Description
As part of changes for #9749, let's rewrite the launcher to Java.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,