Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pack200 issues #4893

Closed
RoiEXLab opened this issue Jun 11, 2019 · 9 comments
Closed

Pack200 issues #4893

RoiEXLab opened this issue Jun 11, 2019 · 9 comments

Comments

@RoiEXLab
Copy link
Member

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
Copy link
Member

(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
Copy link
Member Author

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

@DanVanAtta
Copy link
Member

JMOD overall sounds like a good option

@RoiEXLab
Copy link
Member Author

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
Copy link
Member

@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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

RoiEXLab commented Sep 5, 2019

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.

@RoiEXLab RoiEXLab removed their assignment Sep 5, 2019
@DanVanAtta
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants