Package a backend with the JDK the developer already has - #5859
Merged
Merged
Conversation
`cn1:backend-package` demanded a JDK 8 and refused to run without one, so a
developer on a current JDK was told to install a compiler from 2014 to build a
server. The premise was that a newer javac emits class files the translator
cannot read. It does not: the compile passes `-source 1.8 -target 1.8`, so the
output is class file version 52 whichever javac produces it, and the JavaAPI
bootclasspath that confines a backend to the server-safe surface is enforced
identically -- a `java.nio.file` reference still fails to compile under javac 8,
17, 21 and 25 alike.
`resolveJdk` now takes any JDK 8 or newer and prefers the one already running
Maven, so an ordinary project configures nothing. `-Dcn1.backend.jdk` names
another; `cn1.backend.jdk8` and `JDK_8_HOME` keep selecting one, because
projects and CI jobs were required to set them. Below 8 is refused by reading
`javac -version`, and a javac that has dropped `-source 8` -- 21 and 25 already
warn that it is obsolete -- gets its own wording rewritten with a remedy, since
it names none.
Dropping the demand exposed a second defect it had been hiding.
`JavaSourceCompiler` compiled every generated source in-process with no
`-source`/`-target`, so the class file version was the running JDK's default.
Everything the annotation processors generate is Codename One code that gets
translated -- an app's routers, mappers and bindings through ParparVM, a
backend's entry point through this goal -- and on a JDK 25 the generated
`BackendApplication` came out at version 69 and the translator died with
"Unsupported class file major version 69", naming ASM and a class the developer
never wrote. It now emits Java 8 bytecode by default. An overload takes an
explicit level for the one fixture in the tree that needs a newer language than
8, a record.
Nothing had ever run this goal, which is why the demand went unexamined for as
long as it did: the archetype test stopped at `process-classes` and the platform
builds sit behind their own SDK checks. It now packages the generated backend
natively with `JDK_8_HOME` removed from the environment, starts the binary and
asserts that `/healthz` answers. The workflow installs a JDK 21 for it and
requires clang, because a skipped check reads exactly like a passing one.
Verified end to end rather than by unit test alone: a freshly generated project
packaged on a JDK 25 with `JDK_8_HOME` unset produces a native binary that
answers `/healthz` with `ok` and `/echo?say=works` with `{"say":"works"}`. The
plugin suite is 2393/2393 on JDK 8, 21 and 25, and SpotBugs reports nothing.
Unchanged on purpose: the module's sources are still compiled at Java 8, because
that is the bytecode the translator reads.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
The packaging check added with the previous commit translated perfectly well on the JDK 21 it was given, then failed compiling the generated C on "'curl/curl.h' file not found". clang alone cannot build a backend binary: it links -lcurl -lssl -lcrypto -lnghttp2 -- outbound TLS is libcurl's, the auth primitives are OpenSSL's, the HTTP/2 framing is nghttp2's -- and the natives that call them need those headers to compile at all. The same four packages are installed for the same reason in parparvm-tests.yml, where their absence used to make every test that builds a server binary compile, fail at the link and SKIP, reporting the job green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df8eb631bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// generated source that cannot be translated is of no use to a build. | ||
| public static void compile(Map<String, String> sources, File outputClassDir, List<File> extraClasspath) | ||
| throws IOException { | ||
| compile(sources, outputClassDir, extraClasspath, GENERATED_SOURCE_LEVEL); |
There was a problem hiding this comment.
When Maven runs on a JDK that no longer accepts -source 8, setting -Dcn1.backend.jdk to a compatible JDK does not provide the advertised workaround: the initial backend compilation uses the selected executable, but generateControllers() subsequently reaches this overload, which delegates to ToolProvider.getSystemJavaCompiler() from the JDK running Maven and forces source level 8 again. Projects with generated controllers or DAOs therefore still fail after following the new error message; the generated-source compilation must also use the selected JDK.
Useful? React with 👍 / 👎.
Comment on lines
+72
to
+73
| - name: Install xvfb and clang | ||
| run: bash scripts/ci/apt-get-install.sh xvfb clang |
There was a problem hiding this comment.
The new smoke test always links a backend, but this step installs only clang; the generated C includes <nghttp2/nghttp2.h> and the link command requires curl, OpenSSL, and nghttp2. The same Ubuntu environment is handled in .github/workflows/parparvm-tests.yml by explicitly installing libcurl4-openssl-dev, libssl-dev, and libnghttp2-dev, with a comment noting that clang alone is insufficient. On a runner without those development packages—particularly the nghttp2 headers—the newly required packaging check fails before it can test the JDK behavior.
Useful? React with 👍 / 👎.
Contributor
Cloudflare Preview
|
…ackend Two review findings, one of them a defect in the previous commit's error message. The message told a developer whose javac has dropped `-source 8` to point `-Dcn1.backend.jdk` at one that has not. That property selects the two FORKED steps -- the javac that compiles for translation and the java that runs the translator -- and nothing else. generateControllers compiles the generated router and entry point IN PROCESS, through ToolProvider.getSystemJavaCompiler(), which is the Maven JVM's own compiler and asks for `-source 1.8` as well, so following the advice would have moved the failure one step and no further. The message now names the JDK running Maven, which fixes all three steps at once, and says why the property is not the whole answer. The wording is held there by a test, because a remedy that does not work is worse than none. Routing the in-process compile through the selected JDK would not be an improvement. A release that removes `-source 8` removes it from the Maven JVM too, and this build needs a javac that emits class file version 52 in many more places than one mojo -- codenameone-core still compiles at 1.5 -- so the whole toolchain moves on that day, not this call. That is recorded at JavaSourceCompiler, where the constraint lives. The second finding was the libcurl/OpenSSL/nghttp2 headers, already fixed for the archetype-smoke workflow. It reached `ant.yml` as well, which runs the same script through integration-tests/all.sh and which the previous commit missed: that leg failed on 'curl/curl.h' file not found after translating perfectly well. It gets the same four packages, and CN1_BACKEND_PACKAGE_REQUIRED so the check cannot skip silently there either. It has no CN1_BACKEND_PACKAGE_JDK, so it packages with JAVA_HOME -- a JDK 8 -- which is the other half of the matrix archetype-smoke covers on a JDK 21. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 144 screenshots: 144 matched. |
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
Collaborator
Author
|
Compared 160 screenshots: 160 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
Contributor
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The bug
cn1:backend-packagedemanded a JDK 8 and refused to run without one, so adeveloper on a current JDK was told to install a compiler from 2014 to build a
server.
The premise, written into
resolveJdk8and into the test that guarded it, wasthat a newer javac emits class files the translator cannot read. It does not:
the compile passes
-source 1.8 -target 1.8, so the output is class fileversion 52 whichever javac produces it.
Measured rather than assumed. Compiling
vm/backend/src+impl/parparvmagainst the
vm/JavaAPIbootclasspath under javac 8, 17, 21 and 25 gives thesame 119 classes at major version 52 in every case, with no
invokedynamicstring concatenation on any of them. The bootclasspath still bites identically
too -- a probe referencing
java.nio.fileandjava.util.concurrent.ForkJoinPoolfails to compile under all four, so the property that confines a backend to the
server-safe surface is not weakened by this change.
The fix
BackendPackageMojo--resolveJdktakes any JDK 8 or newer and prefersthe one already running Maven, so an ordinary project configures nothing.
-Dcn1.backend.jdknames another.cn1.backend.jdk8andJDK_8_HOMEkeepselecting one, because projects and CI jobs were required to set them and
dropping a requirement must not break the setups made to satisfy it. Below 8 is
refused by reading
javac -version; a javac that cannot be asked its version isaccepted, because the compile that follows reports what is really wrong with it.
A javac that has dropped
-source 8-- 21 and 25 already warn that it isobsolete -- has its own wording rewritten with a remedy, since it names none.
JavaSourceCompiler-- a second defect the first one was hiding. It compiledevery generated source in-process with no
-source/-target, so the class fileversion was the running JDK's default. Everything the annotation processors
generate is Codename One code that gets translated: an app's routers, mappers and
bindings through ParparVM, a backend's entry point through this goal. On a JDK 25
the generated
BackendApplicationtherefore came out at version 69 and thetranslator died with
Unsupported class file major version 69, naming ASM and aclass the developer never wrote. It now emits Java 8 bytecode by default, and an
overload takes an explicit level for the one fixture in the tree that needs a
newer language than 8 -- a record.
The missing check
Nothing had ever run this goal, which is why the demand went unexamined for as
long as it did:
cn1app-archetype-test.shstopped atprocess-classes, and theplatform builds sit behind their own SDK checks.
It now packages the generated backend natively with
JDK_8_HOMEremoved fromthe environment -- left set, the goal would take that path and the check would
pass while the developer's build still failed -- then starts the binary and
asserts that
/healthzanswers. "The file exists" would have passed on a binarythat cannot start.
archetype-smoke.ymlinstalls a JDK 21 for it and requiresclang, because a skipped check reads exactly like a passing one.
Verification
End to end, not by unit test alone. A freshly generated project packaged on a
JDK 25 with
JDK_8_HOMEunset produces a 5.4MB native binary that answers/healthzwithokand/echo?say=workswith{"say":"works"}.RecordMappingTestcarried a truncated copyright header on master. The gate isdiff-scoped, so editing the file would have failed CI; it now has the complete
one.
Deliberately unchanged
-- once by Maven, once against the JavaAPI bootclasspath -- and ParparVM reads
class file version 52. That is a real constraint, not a leftover.
-source 8is already warned obsolete on 21 and 25, so a JDK that removes it breaks the
archetype's app module too; that deserves its own pass.
mainstill fails translationwith "Multiple main classes". The archetype scopes
sqlite-jdbctoruntime,so it does not bite today. Separate bug on the same goal.
🤖 Generated with Claude Code