Java 11 compatibility - Githubissues

haumacher commented 1 year ago

Java 8 is now really out of date and consuming the ez-vcard dependency is not possible from a modular Java 11 project.

Added module-info to be able to consume ezvcard from a modular Java 11 project.
Updated dependencies required to be able to compile with Java 11.
Removed xalan and tests dependent on xalan since it imports a dependency (xml-apis) that conflicts with the Java 11 module path.
The opens statements are just enough to make the tests run.
All packages are exported, maybe the API could be further restricted?

mangstadt commented 1 year ago

Modular projects can still import non-modular libraries, no?

Android compatibility is also important.

haumacher commented 1 year ago

Modular projects can still import non-modular libraries, no?

No - or only with the workaround of "automodules", where a module is created from a jar based on its file name. This produces the uggly warning during compile:

[INFO] --- maven-compiler-plugin:3.10.1:compile (default-compile)  ---
[WARNING] *******************************************************************************************************************************************
[WARNING] * Required filename-based automodules detected: [ez-vcard-0.11.3.jar]. Please don't publish this project to a public artifact repository! *
[WARNING] *******************************************************************************************************************************************

This workaround is - especially when using Maven-Builds - extremely fragile, because the file name Maven chooses for the library depends on potential clashes with other libraries.

And, what's even worse, while this auto-module thing worked for version 0.11.3 of ez-vcard, it stopped working for version 0.12.0 - I've no idea why, but Eclipse and Maven consistently complain about error: module not found: ez.vcard, when switching from 0.11.3 to 0.12.0.

If Android compatibility is an issue, I would vote for reconsidering proposal #131.

haumacher commented 1 year ago

I updated the PR to again use Java 8 as base version, but adopted the approach in #131 to use the moditect plugin to add a module descriptor.

In contrast to #131, I let Maven auto-generate the descriptor, so that no additional information needs to be maintained in the project. Additionally, I generate a multi-release JAR to minimize the potential of producing conflicts with older target VMs.

mangstadt commented 1 year ago

And, what's even worse, while this auto-module thing worked for version 0.11.3 of ez-vcard, it stopped working for version 0.12.0 - I've no idea why, but Eclipse and Maven consistently complain about error: module not found: ez.vcard, when switching from 0.11.3 to 0.12.0.

One difference between the two versions is that Automatic-Module-Name was added to the JAR manifest in 0.12.0. I'm not sure why that would cause the errors though, because the header is supposed to improve compatibility with modular projects.

Automatic-Module-Name: com.googlecode.ez-vcard

I updated the PR to again use Java 8 as base version, but adopted the approach in https://github.com/mangstadt/ez-vcard/pull/131 to use the moditect plugin to add a module descriptor.

When I try to build the project using a Java 8 JDK, I get this error:

[ERROR] Failed to execute goal org.moditect:moditect-maven-plugin:1.0.0.Final:add-module-info (add-module-infos) on project ez-vcard: Execution add-module-infos of goal org.moditect:moditect-maven-plugin:1.0.0.Final:add-module-info failed: A required class was missing while executing org.moditect:moditect-maven-plugin:1.0.0.Final:add-module-info: java/lang/module/FindException
[ERROR] -----------------------------------------------------
[ERROR] realm =    plugin>org.moditect:moditect-maven-plugin:1.0.0.Final
[ERROR] strategy = org.codehaus.plexus.classworlds.strategy.SelfFirstStrategy
[ERROR] urls[0] = file:/C:/Users/mikea/.m2/repository/org/moditect/moditect-maven-plugin/1.0.0.Final/moditect-maven-plugin-1.0.0.Final.jar
[ERROR] urls[1] = file:/C:/Users/mikea/.m2/repository/org/eclipse/aether/aether-util/1.1.0/aether-util-1.1.0.jar
[ERROR] urls[2] = file:/C:/Users/mikea/.m2/repository/org/moditect/moditect/1.0.0.Final/moditect-1.0.0.Final.jar
[ERROR] urls[3] = file:/C:/Users/mikea/.m2/repository/com/beust/jcommander/1.82/jcommander-1.82.jar
[ERROR] urls[4] = file:/C:/Users/mikea/.m2/repository/org/codehaus/plexus/plexus-utils/1.1/plexus-utils-1.1.jar
[ERROR] Number of foreign imports: 1
[ERROR] import: Entry[import  from realm ClassRealm[project>com.googlecode.ez-vcard:ez-vcard:0.12.1-SNAPSHOT, parent: ClassRealm[maven.api, parent: null]]]
[ERROR]
[ERROR] -----------------------------------------------------

When I try to build the project using a Java 18 JDK, several of the unit tests do not run because of problems with Mockito.

[ERROR] Errors:
[ERROR]   JCardRawReaderTest.bad_snytax:158 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.basic:68 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.complex_value:279 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.data_type_unknown:325 » ExceptionInInitializer
[ERROR]   JCardRawReaderTest.data_type_unrecognized:346 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.different_data_types:252 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.empty:363 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.empty_properties_array:186 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.group:428 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.ignore_other_json:128 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.multi_value:230 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.no_vcard:381 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.parameters:401 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.read_multiple:99 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   JCardRawReaderTest.structured_value:207 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   VCardPropertyTest.validate_overrideable_method_called:72 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3
[ERROR]   IOUtilsTest.closeQuietly:45 » NoClassDefFound Could not initialize class org.mockito.internal.creation.cglib.ClassImposterizer$3

I'm very reluctant to add modular support to this project because of collateral damage it is likely to cause.

haumacher commented 1 year ago

Automatic-Module-Name: com.googlecode.ez-vcard

This is very likely to cause the problem. com.googlecode.ez-vcard is an invalid module name. Module names must follow the Java package syntax, where - is an invalid character. You should either use com.googlecode.ezvcard, or com.googlecode.ez.vcard, or even com.googlecode.ez_vcard.

I built the branch with Java 11, this worked fine. For Java 18, maybe more build tools need an update. But if you just update your module name hint in the manifest, this is could also be good enough. By the way, your dependency vinnie would also benefit from modularization, or at least adding a module name to the JAR (whether this removes the build warnings in dependent projects must be checked).

haumacher commented 1 year ago

I updated the mockito dependency to allow building with newer JDK versions. I'm now able to build with Java 11 and Java 19.

mangstadt commented 1 year ago

Thanks for the tip about Automatic-Module-Name and for fixing the Mockito issue. It's building with JDK 18 now for me. Looks like the latest version of Mockito requires at least Java 11 because I'm getting "class file has wrong version" errors when building it under JDK 8.

Is there a reason why you're specifically looking for Java 11 compatibility? Is it just because that's what your project is using or is Java 11 considered baseline now?

Also, in the moditect plugin configuration, why is Java 11 specifically referenced here? I thought the purpose of this plugin was to make the library compatible with the Java module system in general, no matter what version is being run.

haumacher commented 1 year ago

... why you're specifically looking for Java 11 compatibility

I consider it a good practice for a library or framework to support with its newest version the oldest not yet "very out of date" version of the JDK. Currently, this is Java 11 (EOL 30 Sep 2023), see https://endoflife.date/java.

I also consider it a good practice for libraries to only care about LTS versions of the JDK. Those were Java 8, Java 11 and Java 17. This is why I choose Java 11 as baseline for supporting modular applications in the moditect plugin. Theoretically, one could use Java 9 here, but Java 9 was non LTS and EOL 20 Mar 2018. Nobody should care about this version nowadays.

Auties00 commented 1 year ago

I'm happy to see that someone else brought up this issue, imo it's very important to help the ecosystem transition to the module system. Here are my considerations:

Java 11 is the LTS that came after Java 8, so it supports modules from Java 9
Java 17 is the second LTS after Java 8, it supports modules and it also enforces encapsulation from Java 16(aka illegal reflective operations are no longer legal, even with a JVM flag)
Java 21 will be the third LTS after Java 8 and doesn't introduce anything new in regards to modules as far as we are concerned. It's set to release in October

I'd go for Java 11 in the moditect plugin for maximum compatibility. It should take no more than a couple of minutes in the future to transition to Java 17 or even 21. If it were for me, I'd completely drop support for Java 8: I think that we as open source maintainers have a crucial role in pushing the ecosystem forward, yet I also understand this library is heavily used on Android(gotta love Android's JVM implementation), so i see why maintaining support for older Java versions is crucial.

mangstadt commented 1 year ago

Thanks for popping into the conversation, @Auties00!

I agree that it makes sense to go with Java 11 for maximum compatibility.

I still don't understand why the moditect plugin requires a Java version to be specified. I thought a project either supported modules or doesn't support modules? By telling moditect to use Java 11, are we telling it to support Java 11's version of the module system in particular?

Am I right to assume that the compiled class files will still be compatible with Java 8, even if the project is compiled with a Java 11+ JDK? That's the whole point of using moditect, right?

Thanks for your time with this.

haumacher commented 1 year ago

...why the moditect plugin requires a Java version to be specified...

This requests the plugin to create a "multi-version" JAR, where the Java 11 part is not added directly to the JAR, but encapsulated into META-INF/versions/11/ for maximum compatibility with older versions.

...compiled class files will still be compatible with Java 8...

Yes, since the global target version is still 1.8. You should be able to verify that by adding the JAR as dependency to a Java 8 project, that still compiles with a Java 8 JDK. This is true as long as you do not start using Java API's in ez-vcard that were introduced in a JDK version greater than 8.

mangstadt commented 1 year ago

Yes, since the global target version is still 1.8. You should be able to verify that by adding the JAR as dependency to a Java 8 project, that still compiles with a Java 8 JDK.

Thanks. I've tested the JAR under a non-module project (using JDK 8) and a modular project (using JDK 11) it works with both versions.

It looks like there's another problem. I have been giving users the option of excluding certain dependencies if they do not need to use certain features of ez-vcard. For example, the JSON dependency (Jackson) can be excluded if the user knows they will never need to create JSON-encoded vCards. However, with this new module system, if I exclude a dependency in the POM, I get a runtime error. Any idea how to address this?

Error occurred during initialization of boot layer
java.lang.module.FindException: Module com.fasterxml.jackson.core not found, required by com.googlecode.ezvcard

mangstadt commented 1 year ago

Notes to self:

The Javadoc CSS file must be updated with whatever CSS the JDK you're using to build the project with is using: /src/main/javadoc/syntaxhighlighter.css. The file currently uses the JDK 8 CSS.

The following setting must be added to the Javadoc plugin settings in the POM to prevent a warning related to modules from spamming the screen. See: https://stackoverflow.com/a/62533664/13379

<plugin>    
  <configuration>
    <source>8</source>
  </configuration>
</plugin>

haumacher commented 1 year ago

...the option of excluding certain dependencies...

Optional dependencies have the static modifier in the module-info. I updated the plugin configuration to enforce this modifier, even if the dependency is not marked optional at the Maven level (this would require users to explicitly add the dependency instead of excluding it if not needed).

However, the best solution to such issue would be to split the package into a core not requiring all of those optional dependencies and a set of modules adding additional functionality that actually requires the dependency. Dependency exclusion and optional dependencies are fragile and error-prone workarounds.

... prevent a warning related to modules from spamming the screen ...

I added your suggested configuration option. It seems to help.

... Javadoc CSS file must be updated ...

Don't know what to do here, the generated JavaDoc looks fine to me at the first glance.

mangstadt commented 1 year ago

Optional dependencies have the static modifier in the module-info. I updated the plugin configuration to enforce this modifier, even if the dependency is not marked optional at the Maven level (this would require users to explicitly add the dependency instead of excluding it if not needed).

When I try to write a JSON-encoded vCard in a separate Java 11 project, I get a ClassNotFoundException. Are you saying that the Jackson dependency must be explicitly added to the project's module-info.java file if the user wants to use it?

Also, when I try to create an HTML-encoded vCard (uses the freemarker library), an exception is thrown about it not being able to find the template file on the classpath. The template file is located on the classpath here: /ezvcard/io/html/hcard-template.html. Does the classpath work differently with modular projects?

public class HCardPage {
  public HCardPage() {
    Configuration cfg = ...
    cfg.setClassForTemplateLoading(HCardPage.class, ""); //the template file is in the same package as HCardPage
    Template template = cfg.getTemplate("hcard-template.html"); //exception thrown here
  }
}

Auties00 commented 1 year ago

...the option of excluding certain dependencies...

Optional dependencies have the static modifier in the module-info. I updated the plugin configuration to enforce this modifier, even if the dependency is not marked optional at the Maven level (this would require users to explicitly add the dependency instead of excluding it if not needed).

However, the best solution to such issue would be to split the package into a core not requiring all of those optional dependencies and a set of modules adding additional functionality that actually requires the dependency. Dependency exclusion and optional dependencies are fragile and error-prone workarounds.

... prevent a warning related to modules from spamming the screen ...

I added your suggested configuration option. It seems to help.

... Javadoc CSS file must be updated ...

Don't know what to do here, the generated JavaDoc looks fine to me at the first glance.

I'm pretty sure I got the optional dependencies to work in my PR, you just needed to add the maven dependency explicitly as it always was.

mangstadt / ez-vcard

Java 11 compatibility #135