feat: honor add-opens/exports from jar manifest #1899
Conversation
|
|
||
| // TODO: need to NOT add this if running on java 8 | ||
| // https://openjdk.org/jeps/261#Breaking-encapsulation | ||
| String exports = attrs.getValue("Add-Exports"); |
There was a problem hiding this comment.
@quintesse do you see an easy way we at this point know which java version will be used? or are we at the stage we want to add exports/opens as explicit values on project and only add to runtime options when rendering the launch and info ?
There was a problem hiding this comment.
Unfortunately, the Java version is determined after the importing of the meta data.
The possible solutions I can think of right now are:
- split the importing of metadata into 2 steps, one before and one after the java version has been determined
- filter those options after the java version has been determined
(*) The Java version gets determined in the updateProject() call here: https://github.com/jbangdev/jbang/blob/main/src/main/java/dev/jbang/source/ProjectBuilder.java#L287, the ordering of that and the call to importJarMetadata() can not be changed!
So solutions could either be something like:
return importMoreJarMetadata(prj, updateProject(importJarMetadata(prj, ...)));
return filterNonJava8Options(prj, updateProject(importJarMetadata(prj, ...)));
The downside of the second option is perhaps that it would also filter out options that a user has explicitly added (yes, they shouldn't do that on Java 8, but they asked for it so they perhaps expect an error).
Neither of the two options is particularly elegant.
Edit: btw, the filtering could also be done much later in the process, just before using them, like you said
There was a problem hiding this comment.
A third option I thought of is:
- simply import all manifest entries into the
manifestOptionsmap that already exists and then do what you suggested "only add to runtime options when rendering the launch and info". (Not entirely sure where the best place for that would be)
There was a problem hiding this comment.
manifestOptions - thought that was used for when writing the end jar?
wouldnt it be probelmatic we copy these in ?
or maybe that is actually more what we actually want...
There was a problem hiding this comment.
updated code to put the values into manifestOptions and then put it on command line only when generating jar command.
maxandersen
force-pushed
the
addopensexports
branch
from
12707cf to
5ae9004
Compare
maxandersen
force-pushed
the
addopensexports
branch
from
5ae9004 to
ca0a689
Compare
| } | ||
| String opens = attrs.getValue("Add-Opens"); | ||
| if (opens != null) { | ||
| prj.getManifestAttributes().put("Add-Opens", exports); |
There was a problem hiding this comment.
Indeed. As an alternative , I don't know if it would be useful/better/easier to just blindly copy all options. But that's something we could change at any time in the future.
|
LGTM |
fixes #1852
make it now trivial to run things like:
fish.payara.extras:payara-micro:LATESTandcom.google.googlejavaformat:google-java-format:RELEASEopening in draft as I haven't figured a reliable way to avoid the add opens to be skipped when running on older than java 9.