Skip to content

Conversation

@sgammon
Copy link
Member

@sgammon sgammon commented Jul 5, 2025

Ready for review Powered by Pull Request Badge

Summary

Adds support for a pluggable SPI-based Runner interface, specialized for now to JvmRunner; such runners are capable of accepting JVM-like arguments and running code in a JVM-like manner. There are two initial implementations: EspressoJvmRunner, which uses Truffle and Espresso, and StandardJvmRunner, which either runs code reflectively or via CLI invocation of java.

By default, the StandardJvmRunner will be used, unless any of the following conditions are true:

  1. The user is running tests; in which case, Espresso's instrumentation may be desirable for e.g. coverage
  2. The user passes --jvm:espresso to prefer an Espresso JVM
  3. The standard JVM runner isn't usable for other reasons (e.g. no JAVA_HOME is set, or no java bin is resolvable)
  • Implement Runner Types
    • Add abstract runner implementation
    • Add SPI-based resolution of runners
    • Add JVM runner job and related structures
    • Implement EspressoJvmRunner
    • Implement StandardJvmRunner
      • Reflective (in-proc) execution
      • Subproc execution
  • Implement Runner Support in CLI
    • Add --jvm:espresso flag to prefer a Truffle JVM
    • Resolve a JVM runner and use it instead of running directly
    • Pass/fail command result based on JVM runner outcome

Fixes #1398

@sgammon sgammon added this to the Release R18: Beta milestone Jul 5, 2025
@sgammon sgammon self-assigned this Jul 5, 2025
@sgammon sgammon added feature Large PRs or issues with full-blown features lang:java Issues relating to Java language support labels Jul 5, 2025
@sgammon sgammon added this to Elide Jul 5, 2025
@sgammon sgammon added the lang:kotlin Related to Kotlin lang support label Jul 5, 2025
@sgammon sgammon moved this to In Progress in Elide Jul 5, 2025
@sgammon sgammon force-pushed the feat/run-on-jvm branch from e2304a2 to d4ce0d6 Compare July 5, 2025 21:41
@sgammon sgammon marked this pull request as ready for review July 5, 2025 21:41
@codecov
Copy link

codecov bot commented Jul 5, 2025

Codecov Report

Attention: Patch coverage is 51.83673% with 118 lines in your changes missing coverage. Please review.

Project coverage is 40.52%. Comparing base (bb24f26) to head (594c77f).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...kotlin/elide/tool/cli/cmd/repl/ToolShellCommand.kt 0.00% 42 Missing ⚠️
...tlin/elide/runtime/runner/jvm/StandardJvmRunner.kt 62.50% 19 Missing and 11 partials ⚠️
...main/kotlin/elide/runtime/runner/AbstractRunner.kt 69.44% 8 Missing and 3 partials ⚠️
...n/kotlin/elide/runtime/runner/AbstractRunnerJob.kt 15.38% 10 Missing and 1 partial ⚠️
.../src/main/kotlin/elide/runtime/runner/JvmRunner.kt 64.28% 10 Missing ⚠️
.../main/kotlin/elide/runtime/runner/RunnerOutcome.kt 50.00% 4 Missing ⚠️
...kotlin/elide/runtime/runner/base/RunnerInfoImpl.kt 0.00% 4 Missing ⚠️
...tlin/elide/runtime/runner/jvm/EspressoJvmRunner.kt 78.94% 3 Missing and 1 partial ⚠️
.../kotlin/elide/tool/cli/options/EngineJvmOptions.kt 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1522      +/-   ##
==========================================
+ Coverage   40.41%   40.52%   +0.11%     
==========================================
  Files         704      713       +9     
  Lines       33071    33291     +220     
  Branches     4599     4624      +25     
==========================================
+ Hits        13364    13491     +127     
- Misses      18148    18225      +77     
- Partials     1559     1575      +16     
Flag Coverage Δ
jvm 40.52% <51.83%> (+0.11%) ⬆️
lib 40.52% <51.83%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ner/src/main/kotlin/elide/runtime/runner/Runner.kt 100.00% <100.00%> (ø)
...er/src/main/kotlin/elide/runtime/runner/Runners.kt 100.00% <100.00%> (ø)
.../kotlin/elide/tool/cli/options/EngineJvmOptions.kt 16.66% <0.00%> (-8.34%) ⬇️
.../main/kotlin/elide/runtime/runner/RunnerOutcome.kt 50.00% <50.00%> (ø)
...kotlin/elide/runtime/runner/base/RunnerInfoImpl.kt 0.00% <0.00%> (ø)
...tlin/elide/runtime/runner/jvm/EspressoJvmRunner.kt 78.94% <78.94%> (ø)
.../src/main/kotlin/elide/runtime/runner/JvmRunner.kt 64.28% <64.28%> (ø)
...main/kotlin/elide/runtime/runner/AbstractRunner.kt 69.44% <69.44%> (ø)
...n/kotlin/elide/runtime/runner/AbstractRunnerJob.kt 15.38% <15.38%> (ø)
...tlin/elide/runtime/runner/jvm/StandardJvmRunner.kt 62.50% <62.50%> (ø)
... and 1 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb24f26...594c77f. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sgammon added 2 commits July 5, 2025 14:50
feat(runner): implement espresso-based runner
feat(runner): implement in-proc and subproc jvm runners
test(runner): add tests for jvm runners

Signed-off-by: Sam Gammon <sam@elide.dev>
Signed-off-by: Sam Gammon <sam@elide.dev>
@sgammon sgammon force-pushed the feat/run-on-jvm branch from d4ce0d6 to 0ab0ba4 Compare July 5, 2025 21:51
Comment on lines -77 to +80
versions.java.language = 22
versions.java.language = 21
versions.java.toolchain = 24
versions.java.minimum = 22
versions.java.target = 22
versions.java.minimum = 21
versions.java.target = 21
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@darvld note for IDEA work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only really need to change the target for tooling, core, and base, but this might be better for the sake of consistency

@sgammon sgammon requested review from a team and Copilot July 5, 2025 21:58
@sgammon sgammon force-pushed the feat/run-on-jvm branch from 0ab0ba4 to 1af9355 Compare July 5, 2025 21:58
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a pluggable, SPI-based Runner interface with two JVM implementations and integrates them into the CLI.

  • Add Runner SPI and service loader support.
  • Implement StandardJvmRunner (subprocess or reflective) and EspressoJvmRunner (Truffle/Espresso).
  • Wire up CLI (ToolShellCommand) to resolve and invoke the chosen JVM runner.

Reviewed Changes

Copilot reviewed 33 out of 34 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/runner/src/main/kotlin/elide/runtime/runner/Runners.kt Service‐loader utilities for resolving runners
packages/runner/src/main/kotlin/elide/runtime/runner/jvm/StandardJvmRunner.kt Standard JVM runner implementation
packages/runner/src/main/kotlin/elide/runtime/runner/jvm/EspressoJvmRunner.kt Truffle/Espresso JVM runner implementation
packages/cli/src/main/kotlin/elide/tool/cli/cmd/repl/ToolShellCommand.kt CLI integration for selecting and running JVM jobs
gradle.properties Lowered Java target from 22 to 21
Comments suppressed due to low confidence (2)

packages/runner/src/main/kotlin/elide/runtime/runner/jvm/StandardJvmRunner.kt:115

  • Similar to classpath, the module path is joined with ":". Consider using the platform-specific path separator for modulepath as well.
        addAllStrings(jvmArgs.asArgumentList())

packages/cli/src/main/kotlin/elide/tool/cli/cmd/repl/ToolShellCommand.kt:1296

  • Dispatchers.Default is used but kotlinx.coroutines.Dispatchers is not imported. Add import kotlinx.coroutines.Dispatchers at the top of the file.
      runner.configure(context = unwrap(), coroutineContext = Dispatchers.Default)

@sgammon sgammon force-pushed the feat/run-on-jvm branch from 1af9355 to f5f5429 Compare July 5, 2025 22:21
Copy link
Member

@darvld darvld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit but otherwise great stuff

feat(cli): resolve jvm runner when executing jvm
feat(cli): add `--jvm:espresso` option to prefer truffle
feat(cli): use stock jvm runner by default
feat(cli): prefer truffle when running tests

Signed-off-by: Sam Gammon <sam@elide.dev>
@sgammon sgammon force-pushed the feat/run-on-jvm branch from f5f5429 to 594c77f Compare July 5, 2025 23:40
@sgammon sgammon requested a review from darvld July 5, 2025 23:40
@sgammon sgammon merged commit 11fa54b into main Jul 5, 2025
19 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Elide Jul 5, 2025
@elidebot elidebot mentioned this pull request Jul 19, 2025
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Large PRs or issues with full-blown features lang:java Issues relating to Java language support lang:kotlin Related to Kotlin lang support

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Regular JVM runner compat

3 participants