haraldk / TwelveMonkeys

TwelveMonkeys ImageIO: Additional plug-ins and extensions for Java's ImageIO
https://haraldk.github.io/TwelveMonkeys/
BSD 3-Clause "New" or "Revised" License
1.9k stars 314 forks source link

Make the project Java 9+ compatible (modules) #292

Open aromanelli opened 8 years ago

aromanelli commented 8 years ago

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

haraldk commented 8 years ago

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

jansohn commented 7 years ago

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

haraldk commented 7 years ago

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

j-p-sequeira commented 6 years ago

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

haraldk commented 6 years ago

@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

j-p-sequeira commented 6 years ago

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 😃

haraldk commented 5 years ago

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

elect86 commented 4 years ago

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..

haraldk commented 4 years ago

@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

elect86 commented 4 years ago

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

haraldk commented 4 years ago

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

elect86 commented 4 years ago

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?)

Schmidor commented 4 years ago

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".

elect86 commented 4 years ago

Ah, pity..

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

haraldk commented 4 years ago

@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

elect86 commented 4 years ago

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

tmccombs commented 1 year ago

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