TwelveMonkeys icon indicating copy to clipboard operation
TwelveMonkeys copied to clipboard

Make the project Java 9+ compatible (modules)

Open aromanelli opened this issue 7 years ago • 17 comments

Is TwelveMonkeys compatible with Java 9? Any private APIs that used that will break with module loading/jigsaw?

aromanelli avatar Nov 10 '16 00:11 aromanelli

Hi Adrian,

I'm currently at Devoxx, Belgium, learning all about JDK 9/Jigsaw and modularization. ;-)

There should be no problem using the current binaries, as far as I can see. You can choose to just use class path as before. Or you can choose to create module info for the existing jars, and use as modules.

I suspect there might be some trouble building one test module (imageio-reference) on JDK 9, but the project doesn't currently build (tests fails) on OpenJDK anyway. Feel free to play around and report your findings! :-)

The module system seems like a good idea, so I'll probably modularize the project at some point. At the same time, I want to retain JDK 7 compatibility for some time still. That needs to be addressed before we can make the switch.

Harald K

haraldk avatar Nov 10 '16 09:11 haraldk

So the inclusion of the TIFF image format (http://openjdk.java.net/jeps/262) won't be a problem?

jansohn avatar Sep 08 '17 09:09 jansohn

Should not be a problem. In some cases the the JRE bundled TIFF plugin might be selected instead of the TwelveMonkeys one though, which may not be desirable, unless we update the Spi classes. But that's not really a compatibility issue.

See also #298

haraldk avatar Sep 08 '17 12:09 haraldk

My application (compiled with Java 8) running on Java 9, works really well besides this warning:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.twelvemonkeys.imageio.plugins.tiff.TIFFImageReader (file:/C:/Program%20Files/Meew/lib/imageio-tiff-3.3.2.jar) to constructor com.sun.imageio.plugins.jpeg.JPEGImageReader(javax.imageio.spi.ImageReaderSpi)
WARNING: Please consider reporting this to the maintainers of com.twelvemonkeys.imageio.plugins.tiff.TIFFImageReader
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

By the way, what is the best way to contact haraldk? I'd really love to speak with him

j-p-sequeira avatar Dec 14 '17 11:12 j-p-sequeira

@j-p-sequeira: I believe the warning you mention is resolved already in the lastest master. We no longer create the JPEG reader delegate by reflection, but instead using standard ImageIO. This has the added benefit of using our own JPEGImageReader (if available), which supports CMYK and JPEG Lossless among other things.

If you want to contact me for anything related to the project, please use this issue tracker.

For personal email, you may use harald dot kuhr, at gmail (com).

Best regards,

-- Harald K

haraldk avatar Dec 14 '17 12:12 haraldk

I believe the warning you mention is resolved already in the lastest master.

It may be so, I'm using the JARs (version 3.3.2) that I downloaded back in April or May and I noticed that some changes have been made since then.

And it's nice to meet you, thank you for the contact info, I'll "bother" you sometime soon 😃

j-p-sequeira avatar Dec 14 '17 16:12 j-p-sequeira

Related: https://stackoverflow.com/q/53477690/1428606

haraldk avatar Nov 26 '18 09:11 haraldk

I get tons of packages split errors (such as error: module imageio.icns reads package javax.xml.transform from both xml.apis and java.xml)

Any news about JPMS compatibility?

Edit: apparently the problem shows up as soon as I include -batik and the corresponding apache transcoder library..

elect86 avatar Mar 10 '20 15:03 elect86

@elect86

JPMS compatibility is certainly on the list of things I'd like to do, however, time budget (as it is, with no sponsors) is very limited.

I'm not 100% sure how the automatic module stuff (for non-JPMS JARs) work in detail, but I believe the problem is that xml-apis.jar contains the javax.xml.transform package (which is also part of the java.xml module). Perhaps we could change the dependency to provided/optional or find a more recent version that don't have this problem? Other than that, I belive this specific problem will have to be fixed in xml-apis.jar regardless of proper JPMS support in TwelveMonkeys.

Best regards,

-- Harald K

haraldk avatar Mar 11 '20 13:03 haraldk

JPMS compatibility is certainly on the list of things I'd like to do, however, time budget (as it is, with no sponsors) is very limited.

You wouldnt believe me how much I understand you..

I'm not 100% sure how the automatic module stuff (for non-JPMS JARs) work in detail, but I believe the problem is that xml-apis.jar contains the javax.xml.transform package (which is also part of the java.xml module). Perhaps we could change the dependency to provided/optional or find a more recent version that don't have this problem? Other than that, I belive this specific problem will have to be fixed in xml-apis.jar regardless of proper JPMS support in TwelveMonkeys.

Then is xml-apis to blame, which version are you using exactly? (this might be relevant)

Also, may I suggest to offer a -all alternative so that I dont have manually type every single module?

Moreover, if you want, I can do a PR and add in README a Gradle snippet to quickly copy/paste

elect86 avatar Mar 11 '20 13:03 elect86

A quick search reveals that I have no direct dependency on xml-apis, whoever, Batik does.

batik-extension-1.9.pom (and other Batik 1.9 POMs) contains:

    <dependency>
      <groupId>xml-apis</groupId>
      <artifactId>xml-apis</artifactId>
      <version>1.3.04</version>
    </dependency>
    <dependency>
      <groupId>xml-apis</groupId>
      <artifactId>xml-apis-ext</artifactId>
      <version>1.3.04</version>
    </dependency>

We might have some luck upgrading to a later version of Batik, if you want to dig into that. I'm planning to do an upgrade soon anyway, due to #526 .

Also, I'm not sure I fully understand what you mean by an -all option... Do you mean a single JAR containing everything? I don't plan to add that. I do have a BOM you can depend on to avoid declaring lots of dependencies in your POM or Gradle project though..

-- Harald K

haraldk avatar Mar 11 '20 13:03 haraldk

I can try to upgrade.. 1.12 seems to be the last one

Which is the fastest way to test it? I'm not super familiar with maven (on the same wave, how can I use the BOM on Gradle?)

elect86 avatar Mar 11 '20 14:03 elect86

Batik 1.12 still has those dependencies. xml-apis could be ignored as it is already included in the jre since a very long time. Unfortunately xml-apis-ext is still required and uses the same packages as the java module "java.xml".

Schmidor avatar Mar 11 '20 14:03 Schmidor

Ah, pity..

then I dont see any easy solution other than remove batik support for the moment..

elect86 avatar Mar 11 '20 14:03 elect86

@elect86 I don't use Gradle (or the BOM) myself, but I assume you just depend on com.twelvemonkeys.bom:bom instead of the individual modules.

@Schmidor Thanks for checking that out! Even if that means there's no easy fix.. :-(

PS: You don't HAVE to use modules (JPMS) even if you are using a more recent Java version. But I assume you have reasons to do that?

-- Harald K

haraldk avatar Mar 11 '20 15:03 haraldk

Yeah, I know, the last goal is to make our end-user application modular, however this is actually prevented by not-first class JPMS support in Gradle + Kotlin

But sooner or later we'll got there

elect86 avatar Mar 13 '20 09:03 elect86

I get this from the imageio-bmp tests:

[ERROR] WARNING: An illegal reflective access operation has occurred
[ERROR] WARNING: Illegal reflective access by com.twelvemonkeys.imageio.plugins.bmp.BMPImageReaderTest (file:/home/thayne/dev/TwelveMonkeys/imageio/imageio-bmp/target/test-classes/) to constructor com.sun.imageio.plugins.bmp.BMPImageReader(javax.imageio.spi.ImageReaderSpi)
[ERROR] WARNING: Please consider reporting this to the maintainers of com.twelvemonkeys.imageio.plugins.bmp.BMPImageReaderTest

tmccombs avatar Jan 11 '23 16:01 tmccombs