Conversation
f6e5270 to
6f4602f
Compare
a70125f to
952fd82
Compare
37903d9 to
f39a36a
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| classpath = ":".join([":".join([ f'"{d}"' for d in deps ]), ":".join([ f'"{p}"' for p in plugins ])]) | ||
| cmd = f"java -cp {classpath} -Djava.library.path={jep_path} polynote.Main {' '.join(sys.argv[1:])}" | ||
| java_executable = os.path.join(os.environ['JAVA_HOME'], 'bin', 'java') if os.environ.get('JAVA_HOME') else 'java' | ||
| cmd = f"{java_executable} -cp {classpath} -Djava.library.path={jep_path} polynote.Main {' '.join(sys.argv[1:])}" |
There was a problem hiding this comment.
This would give us flexibility to launch polynote with desired jdk during apt-get install polynote
f39a36a to
fe66502
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the Java runtime from JDK 8 to JDK 17 across the project's build and execution environments.
- Updates all GitHub Actions workflows to use Java 17 instead of Java 8
- Modifies the Python launcher script to support custom JAVA_HOME configurations
- Ensures consistent Java version across CI pipelines and distribution builds
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/polynote.py | Adds JAVA_HOME environment variable support for custom JDK paths |
| .github/workflows/dist.yml | Updates distribution workflow to use Java 17 |
| .github/workflows/ci-backend-2.13.yml | Updates Scala 2.13 CI pipeline to use Java 17 |
| .github/workflows/ci-backend-2.12.yml | Updates Scala 2.12 CI pipeline to use Java 17 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jonathanindig
left a comment
There was a problem hiding this comment.
Based on my understanding this isn't changing anything compatibility-wise as it'll still be generating Java 8 compatible bytecode (which is fine though we will probably want to update that eventually). Please lmk if that's not the case - otherwise, LGTM.
Please add a description before merging, thanks!
fe66502 to
e421d30
Compare
e421d30 to
900679b
Compare
2daacfd to
7922bc2
Compare
202847b to
6f4c37a
Compare
That is correct. Scala 2.12.x can only produce JDK 8 bytecode anyway. For 2.13.x, not sure, but may not matter too much because there is no prod usage yet. |
af9954b to
ffbddaf
Compare
f0ee547 to
65b6b10
Compare
7c6c8b5 to
24463d8
Compare
Merge activity
|
Summary
Upgrades Polynote to use Java 17 (from Java 8) for CI builds, adding necessary JVM compatibility flags for Spark.
Changes
Java Version Upgrade
ci-backend-2.12.yml: Java 8 → 17ci-backend-2.13.yml: Java 8 → 17dist.yml: Java 8 → 17Spark Java 17 Compatibility
build.sbtfor Spark tests to work with Java 17+--add-opensfor: java.lang, java.io, java.net, java.nio, java.util, sun.nio, sun.security, and moreScript Improvements
polynote.pyto useJAVA_HOMEif set (previously always usedjavafrom PATH)Compatibility
✅ Benefits
📋 Requirements
Technical Details
JVM Flags Added:
-XX:+IgnoreUnrecognizedVMOptions- Gracefully handle unknown options--add-opens=java.base/*=ALL-UNNAMED- Opens internal Java modules for Spark's reflection/unsafe usageWhy needed:
sun.misc.Unsafeand reflection on internal classesInaccessibleObjectException