search

triplea-game / triplea

TripleA is a turn based strategy game and board game engine, similar to Axis & Allies or Risk.
https://triplea-game.org/
GNU General Public License v3.0
1.34k stars 393 forks source link

Pack200 issues #4893

Closed RoiEXLab closed 4 years ago

RoiEXLab commented 5 years ago

Since the java 11 upgrade during deployment the pack200 tool invoked by install4j is spamming the console with warnings:

Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/TriangleMeshBuilder.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/ProxyBuilder$1.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/ProxyBuilder$AnnotationValue.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/JavaFXSceneBuilder.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/ProxyBuilder$Property.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/URLBuilder.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/JavaFXFontBuilder.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/ProxyBuilder$Getter.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/ProxyBuilder$ArrayListWrapper.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/ProxyBuilder.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/JavaFXImageBuilder.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/builder/ProxyBuilder$Setter.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/MethodHelper.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/ParseTraceElement.class
Jun 11, 2019 6:34:30 PM com.sun.java.util.jar.pack.Utils$Pack200Logger warning
WARNING: Passing class file uncompressed due to unrecognized attribute: com/sun/javafx/fxml/BeanAdapter$MethodCache.class

(There are a lot more, the 10000 lines threshold is exeeded).

I'm not entirely sure if this is an issue with Pack200 + JavaFX, or just with install4j that fails to pick the correct version for the tool (unlikely because I only see JavaFX classes, but whatever).

However the Pack200 tool is deprecated and marked for removal. Detailed information can be found here. TL;DR; The OpenJDK community is basically saying, hey pack200 was nice in the past, but it's legacy code and we don't use it anymore because we now have the JMOD format (instead of JAR) which already compresses the content. So it might be worth turning off Pack200 (since the installer is compressed anyways), and hope that https://openjdk.java.net/jeps/343 will be implemented in the near future (looks like some code was pushed recently, so there's definitely progress), so we can use the "native" packaging tools and might not even need to rely on install4j anymore. Depending on your point of view it might be worth trying to fix the pack200 setup though, even if it isn't a long term solution. I'd be happy to hear your thoughts on this one.

DanVanAtta commented 5 years ago

(1) we probably should suppress that log message with something like:

COMMAND | egrep -v "Passing class file uncompressed due to unrecognized attribute|Pack200Logger warning"

(2) install4j seems to be working for now, so I don't think there is a rush, using a native tool and dropping the install4j dependency is not bad. Does JMOD bundle additional files beyond class files? For example there are the icon images that we add to packages.

RoiEXLab commented 5 years ago

@DanVanAtta AFAIK JMOD has a standardised way to store classes, resources, and native binaries

DanVanAtta commented 5 years ago

JMOD overall sounds like a good option

RoiEXLab commented 5 years ago

Yeah, but getting there will probably require us to convert the codebase to use modules. Not sure if this also includes having to declare modules for dependencies

DanVanAtta commented 5 years ago

@RoiEXLab is there a current action item we can tackle now? Seems we will want to get on to JMOD at some point for the installer size benefit. At this point I think the pack200 warnings problem has been mitigated. If so perhaps we can close this issue and let pack200 die a natural death when we migrate away from it.

RoiEXLab commented 5 years ago

Well if we agree on targeting JMOD deployment , we could start by creating module-info.java files that only import required modules from the standard library. This is a fairly reasonable task and should be doable in a day. It raises 2 questions though:

  1. How should the modules be defined? One java 9 module per gradle subproject? Or should we group similar packages together?
  2. How do we figure out JRE dependencies of our dependencies that haven't migrated to modules? There's a mechanism to define the module-info.java on your own for your "legacy" dependencies, and jdeps a tool to figure out the dependencies for you but I don't know how to use them efficiently
DanVanAtta commented 5 years ago

Good questions, sounds like this issue is now more about migrating to JMOD. Is it a long term or a medium goal?

Java9 module per subproject sounds reasonable. 'game-core' is still mostly not broken out, what would it look like for similar packages to be grouped together?

RoiEXLab commented 5 years ago

Good questions, sounds like this issue is now more about migrating to JMOD. Is it a long term or a medium goal?

Good question, depends on us really. If somebody wants to spend a week dealing with all of this new technologies, then this could be very much a short-term goal, however if common tooling doesn't work out for us, then this could be a very tedious process.

what would it look like for similar packages to be grouped together?

AFAIK you can specify a module-info.java in a root package, and this module will contain all subpackages, so if we had all UI packages in one place for example, we could import UI modules only for this part. Obviously if other parts of the codebase depend on this module, they have an indirect dependency on UI modules as well.

DanVanAtta commented 4 years ago

The warning text in travis log is truncated/mitigated. Seems our other plans to fix this issue are long term. Closing this issue as we have no active action items that would allow us to resolve this problem.