JodaOrg / joda-time

Joda-Time is the widely used replacement for the Java date and time classes prior to Java SE 8.
http://www.joda.org/joda-time/
Apache License 2.0
4.98k stars 986 forks source link

Support Java Modules #689

Open aalmiray opened 1 year ago

aalmiray commented 1 year ago

joda-time 2.12.2 defines an automatic module name

$ jarviz module descriptor --file joda-time-2.12.2.jar 
name: org.joda.time
version: 2.12.2
open: false
automatic: true
requires:
  java.base mandated
contains:
  org.joda.time
  org.joda.time.base
  org.joda.time.chrono
  org.joda.time.convert
  org.joda.time.field
  org.joda.time.format
  org.joda.time.tz

I'd be great if the library supplied a full Java module descriptor. It's possible to keep bytecode baseline compatible with Java 5 while providing a full module descriptor thanks to ModiTect. This will help modular projects that consume joda-time, specifically those that create custom Java Runtimes with jlink, as the latter does not support automatic modules but explicit modules. If interested I can send a PR to make it happen.

jodastephen commented 1 year ago

My experience is that adding module-info then results in a slew of complaints that the code is not compatible, such as with Android. For two other projects I then had to add a classier release that excludes module-info. That jlink does not support automatic modules is something that should be taken up with the authors of jlink (or by providing a moditect wrapper for jlink).

aalmiray commented 1 year ago

Fair. Does the incompatibility occur if module-info.class is placed under /META-INF/versions/9? Because that's what I was thinking with ModiTect. Adding module-info.class to the unversioned class space is just silly, lazy, and for some a result of not knowing the rules for multirelease JARs. Thankfully ModiTect does know the rules.

We know the authors of jlink are not going to move regarding support for automatic module names (which in effect are like explicit modules that export everything by default, argh...).

jodastephen commented 1 year ago

I have no experience with multi-version jars, but the issues were with tooling that ignored modules, so I imagine they would ignore multi-release jars too and still break. Put simply, my experience with modules is pretty much entirely negative. Given there are thousands of other libraries out there without module-info, and given how widely joda-time is used, I don't see why its a good choice I'm afraid. There are huge risks here.

bowbahdoe commented 1 year ago

@jodastephen

but the issues were with tooling that ignored modules, so I imagine they would ignore multi-release jars too and still break

META-INF/ was always a special "anything goes" section of a .jar file. I don't think even outdated tooling would dig in there.

I think the issues came from the place @aalmiray suggested. (I also have a vague memory of android issues)

If a jar looked like this

example/
   Math.class (Java 5)
module-info.class (Java 9)

Then tooling can get confused.

But if a jar looks like this

example/
   Math.class (Java 5)
META-INF/
  versions/
    9/
      module-info.class (Java 9)
   MANIFEST.MF

Then there really shouldn't be any issues.

If you could find maybe one of the issues you remember?

Given there are thousands of other libraries out there without module-info, and given how widely joda-time is used, I don't see why its a good choice I'm afraid.

I think thats why it would be valuable. There are tons of libraries that are not jlink compatible. More than 95% of them i'd wager.

Part of the problem is that to be jlink compatible, you need all your dependencies to be as well. joda-time as a leaf on a lot of depends trees. We can't get modularized aws-sdk unless this and a few other apache-commons projects add module-infos.

There are huge risks here.

I think that if we dig into your negative experiences we can put them in boxes and make sure we don't make the same mistakes here.

jodastephen commented 1 year ago

See https://github.com/JodaOrg/joda-convert/issues/24. I had to create a dedicated jar file without module-info.class. And IIRC it made no difference whether it was in META-INF or not. I have no desire to do that for joda-time. Remember, the module system only brings negatives for library authors like myself. A modular jar file can be used on the classpath or modulepath and can behave differently, so you get more edge cases and more things to test.

As I've said before, what you want is to fix or fork jlink. Or write a preprocessor. The task of updating all maven modules in the universe to add module-info simply isn't possible (many projects are dead for a start).

headius commented 10 months ago

I just noticed the v3 label added recently. Is the tide turning?

I just managed to get a fully-modularized JRuby to load and boot all of its guts, run Ruby code, load gems, etc, and I figured the next step was to try to jlink it. Bam, joda-time stopped me dead in my tracks.

We have tried many times to migrate to the new JDK time API, but the translation from joda-time is difficult and we have been unable to accomplish it. So we are stuck with joda-time for the foreseeable future. If we can't load it as a module, it's going to prevent us taking advantage of some of the best features of modularity.

What can I do to help move this forward?

jodastephen commented 10 months ago

Producing a v3 of Joda-Time is a lot of work, but it is reaching the point where it may be required. Such a release would include issues like:

But the existing codebase will have to be maintained on Java 8, so it is effectively doubling project maintenance.

jodastephen commented 10 months ago

If you'd like to help, the first thing I need to know is whether the v3.x branch of Joda-Convert causes issues to JRuby/jlink. https://github.com/JodaOrg/joda-convert/commits/v3.x/ . A new version of Joda-Convert is a prerequisite to a new version of Joda-Time.

I've altered the module definition to requires static Guava and ThreeTen-Backport, both of which are automatic modules without a module-info.class. Does this work or cause issues? (The previous approach relied on reflection to load the jar files and add a read-edge at runtime, which was unpleasant and probably didn't actually work)

aalmiray commented 10 months ago

Those 2 additional dependencies would need explicit module descriptors if joda-convert were to be used when creating a modular runtime with jlink.

I do recall an open issue in Guava where the use of explicit modules was requested and the team was leaning into it. However, v33 has yet to add explicit modules, just checked and it is still automatic.

jodastephen commented 10 months ago

Joda-Time would only optionally need Joda-Convert (requires static). Does that help given most Joda-Time users don't need Joda-Convert?

headius commented 10 months ago

Joda-Time would only optionally need Joda-Convert

If I can ship JRuby with only Joda-Time and the module subsystem is ok with that, it would be fine with me. But I need Joda-Time itself to have a proper module-info, I believe.

I'm not opposed to making a very soft fork for JRuby that just adds a module-info. I don't really want to maintain a fork, but it would only ever update when we decide to update from main, and I'd just be merging periodically.

I sympathize with your concerns, honestly. I have not attempted to tackle this problem until now, because we are planning to require Java 17 minimum in our next major JRuby release. I gave up trying to solve modularity while we still supported Java 8.

headius commented 10 months ago

fork

Another option would be to make a modularized-joda-time that shades in joda-time and just adds the module-info. Then the only periodic update would be the joda-time version and push a new artifact.

headius commented 10 months ago

Another option would be for us to just keep shading any libraries that don't publish a module-info. It's not ideal but it would work.