fix(run): honor manifest launcher flags#2439
Conversation
…butes Extends manifest attribute support for jars beyond PR jbangdev#2439 to handle Add-Reads module dependencies and SplashScreen-Image extraction. Changes: - Add Project.ATTR_ADD_READS and Project.ATTR_SPLASH_SCREEN_IMAGE constants - Import Add-Reads and SplashScreen-Image attributes from jar manifests - Process Add-Reads: converts to --add-reads JVM flags (Java 9+) - Process SplashScreen-Image: extracts image and passes -splash flag - Fix KeyValue.of() to support values containing '=' signs Add-Reads implementation: - Parses space-separated module dependencies (e.g., "mod1=mod2 mod3=mod4") - Generates --add-reads flags for each dependency pair - Version-gated for Java 9+ with runAsModule check SplashScreen-Image implementation: - Extracts splash image from jar to <jarPath>.splash.<ext> in cache - Smart caching: only re-extracts if jar is newer than cached image - Fails gracefully with warnings (never breaks the build) - Adds -splash:<path> flag before other JVM options KeyValue parser fix: - Changed split("=") to split("=", 2) to handle values with '=' - Enables manifest directives like //MANIFEST Add-Reads=mod1=mod2 This builds on PR jbangdev#2439's copyManifestAttribute() and addAllUnnamedManifestOptions() patterns. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…butes Extends manifest attribute support for jars beyond PR jbangdev#2439 to handle Add-Reads module dependencies and SplashScreen-Image extraction. Changes: - Add Project.ATTR_ADD_READS and Project.ATTR_SPLASH_SCREEN_IMAGE constants - Import Add-Reads and SplashScreen-Image attributes from jar manifests - Process Add-Reads: converts to --add-reads JVM flags (Java 9+) - Process SplashScreen-Image: extracts image and passes -splash flag - Fix KeyValue.of() to support values containing '=' signs Add-Reads implementation: - Parses space-separated module dependencies (e.g., "mod1=mod2 mod3=mod4") - Generates --add-reads flags for each dependency pair - Version-gated for Java 9+ with runAsModule check SplashScreen-Image implementation: - Extracts splash image from jar to <jarPath>.splash.<ext> in cache - Smart caching: only re-extracts if jar is newer than cached image - Fails gracefully with warnings (never breaks the build) - Adds -splash:<path> flag before other JVM options KeyValue parser fix: - Changed split("=") to split("=", 2) to handle values with '=' - Enables manifest directives like //MANIFEST Add-Reads=mod1=mod2 This builds on PR jbangdev#2439's copyManifestAttribute() and addAllUnnamedManifestOptions() patterns. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
… helper methods - Replace copyManifestAttribute() and addAllUnnamedManifestOptions() with inline code - These helper methods are from PR jbangdev#2439 which is not yet merged - Use the same pattern that exists in main branch - Add missing imports for JarOutputStream, Manifest, and assumeTrue in tests - Fix test code to use correct CmdGenerator.builder() pattern Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
| optionalArgs.add("--add-opens=" + val + "=ALL-UNNAMED"); | ||
| } | ||
| } | ||
| if (!runAsModule && jdk.majorVersion() >= 9) { |
There was a problem hiding this comment.
is not runAsModule a requirement here ? add-opens and add-exports still apply wether it is run as a module or not is it not?
Got example where this will be wrong/fail?
There was a problem hiding this comment.
Sorry for that that's my mistake. I don’t really have a real case to justify the runAsModule guard, so I’ll remove it
maxandersen
left a comment
There was a problem hiding this comment.
overall direction is good!
But not following why run-as-module should affect these settings; if its related to module-info presence then I would like example/test of what breaks/fails
and jbang should not fail on attributes it does not know if java -jar would not fail so need that to be gracefully handled or shown case where -jar would fail
8ab21ec to
b10580d
Compare
b10580d to
e35dd51
Compare
Thanks for the review @maxandersen |
Changes:
- Replace validation with pass-through for Enable-Native-Access
- Let JVM validate values instead of JBang
- Consistent with how we handle Add-Opens/Add-Exports
- Simpler code, future-proof if spec changes
- Add addManifestOptions() helper
- Similar to addAllUnnamedManifestOptions but without =ALL-UNNAMED suffix
- Used for Enable-Native-Access which passes values directly
- Update tests:
- Replace testRejectsInvalidEnableNativeAccess with testPassesThroughEnableNativeAccessModuleName
- Add testReadingAddExportsWithHelper (using new createJar() helper)
- Add edge case tests:
* testEmptyManifestAttributeIgnored (whitespace-only values)
* testMultipleSpacesBetweenValues (multiple spaces between values)
* testMultipleModulesForEnableNativeAccess (space-separated module list)
- Add documentation:
- New "JAR Manifest Attributes" section in running.adoc
- Documents Add-Opens, Add-Exports, Enable-Native-Access
- Explains how values are passed to JVM
- Links to JAR spec and JEP 472
- References issue jbangdev#2441 for --ignore-manifest flag
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
maxandersen
left a comment
There was a problem hiding this comment.
i changed to simple passthrough for now to let javac complain + added tests/docs
|
@all-contributors add @@ayagmar for code |
|
Could not find the user |
|
https://github.com/all-contributors add @ayagmar for code |
Fixes #2366 and #2438.
JBang runs jars with
-classpath ... Maininstead ofjava -jar, so manifest launcher entries were being ignored. This change reads the relevant values from the primary jar manifest and passes the equivalent JVM flags when JBang launches the jarThat means:
Add-Opens->--add-opens=...=ALL-UNNAMEDAdd-Exports->--add-exports=...=ALL-UNNAMEDEnable-Native-Access: ALL-UNNAMED->--enable-native-access=ALL-UNNAMEDThis also fixes a bug where
Add-Openswas being read from the wrong manifest value.Added regression tests for the
Add-OpensandEnable-Native-Accesscases.Not included : the separate classpath duplication issue mentioned in #2438.