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

Deprecate public APIs referencing Joda Time #6602

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

headius
Copy link
Member

@headius headius commented Mar 8, 2021

This will be a series of commits to deprecate and replace all public APIs referencing classes from the Joda Time library and providing new versions based on java.time.

We will need to maintain these deprecated methods for a while before we can remove them and Joda Time from JRuby.

Some helpful guides:

@headius headius added this to the JRuby 9.3.0.0 milestone Mar 8, 2021
@headius
Copy link
Member Author

headius commented Mar 8, 2021

@kares I think you may have looked into a full conversion in the past, perhaps you know how to handle some of the other APIs we have that use Joda types?

@headius headius linked an issue Mar 8, 2021 that may be closed by this pull request
@kares
Copy link
Member

kares commented Mar 9, 2021

@headius great effort, what I ended up with with was pretty much the same as you - did not find direct replacement for JRuby's various use-cases of JODA DateTime. in some places it made sense to use ZonedTime but (e.g. in RubyTime although sometimes it's really a LocalTime we want), while others (RubyDate/RubyDateTime) seemed to need something different.

have you looked at what TruffleRuby is using in the backend for the Ruby date/time types - they weren't using Java 8 time APIs back when I looked but now they should be?

@headius
Copy link
Member Author

headius commented Mar 9, 2021

@kares Last time I looked they were using the Java 8 time API and ZonedDateTime. Do you remember any specific examples of behavior that needed local time instead? I am not planning to carry this conversion forward for 9.3 but we'll need to do it at some point.

@headius
Copy link
Member Author

headius commented May 18, 2021

I have pushed my last changes, which deprecate all remaining public-facing APIs with Joda classes and suppress or fix deprecation warnings internally to JRuby.

There will be some churn using the new java.time APIs since we still use Joda internally but this at least provides a migration path for third-party libraries. We can incrementally convert the internals to java.time in point releases leading up to a complete conversion by 9.4.

@headius headius requested review from enebo and kares and removed request for enebo May 18, 2021 20:10
@headius
Copy link
Member Author

headius commented May 19, 2021

This is getting punted to 9.4, sadly.

The last couple commits led me to discover that java.time's time zone support is significantly stricter and narrower than the time zone support in Joda. It may not be possible to do a direct conversion from Joda to java.time without extending how java.time deals with custom or legacy time zone specifications. There may be time zone info providers out there for java.time that help fill in the gaps, but for now it is not possible to proceed with this conversion.

A couple examples:

  • A TZ of "AST-3:00:00" should be treated as a time zone named "AST" with an offset of +3, but that does not correspond to any standard time zone ("AST" in java.time is Anchorage Standard Time at -10). As a result, we can only represent it in java.time as a ZoneOffset of +3 with no name.
  • A TZ of "PST8:00:00" should be treated as a time zone named "PST" with an offset of -8, which matches Pacific Standard Time, but when parsed in this way it ends up as a nameless zone at -8.
  • A TZ of "PDT" should be treated as Pacific Daylight Time but this ID does not roundtrip through java.time without error.

I believe now we will have to attempt the conversion from the inside out, matching behavior using java.time before we decide that we can ditch Joda. This won't happen for 9.3, and since we do not yet know whether we are able to use java.time to represent Ruby Time and Date, we cannot deprecate the Joda APIs.

Joda Time is being deprecated in favor of the Java 8 java.time API
and we need to start moving away from the former. This patch marks
all public APIs that accept or return Joda DateTime as deprecated
and provides java.time.ZonedDateTime versions for migration. This
patch does NOT convert any internals to using java.time APIs.

There are other Joda classes referenced in public APIs. so this is
only part of the work needed for jruby#6600.

I have not suppressed deprecation warnings so we can see where we
still call the deprecated public APIs in JRuby.

This makes use of the MIT-licensed "timeywimey" converted class,
see https://github.com/meetup/timeywimey.
This deprecates the remaining set of user-facing Joda Time-based
JRuby APIs:

* DateTime APIs are replaced with ZonedDateTime equivalents
* DateTimeZone APIs are replaced with ZoneId equivalents
* Chronology APIs are deprecated with no replacement

All internal uses of these deprecated APIs have either been moved
to the new java.time versions or supppress deprecation warnings.
* Handle a null DateTimeZone as we did before, by propagating the
  null. This is used to indicate an invalid zone in our Ruby Time
  logic.
* Provide a map of zone aliases to pick up the old three-letter
  zones. This helps but is not sufficient to handle all
  three-letter formats tested by specs and tests (e.g. "PDT").
* For fixed-offset zones, use the offset rather than the ID. This
  causes those zones to lose their ID, so "AST-3:00:00" becomes
  an unnamed ZoneOffset of "3:00:00". This leads to other failures
  when the ID and offset are both expected to propagate into Time.
@headius
Copy link
Member Author

headius commented Nov 3, 2022

I have rebased this on latest master but the problems remain: java.time is not a drop-in replacement due to strict and limited time zone support, so even if we deprecate the Joda Time APIs we can't start using java.time internally yet.

I find the java.time API to be inscrutable so I'm not sure how to proceed.

@headius headius modified the milestones: JRuby 9.4.0.0, JRuby 9.5.0.0 Nov 10, 2022
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

Successfully merging this pull request may close these issues.

Deprecate Joda APIs and replace with Java 8 API
2 participants