FasterXML / jackson-modules-java8

Set of support modules for Java 8 datatypes (Optionals, date/time) and features (parameter names)
Apache License 2.0
399 stars 116 forks source link

run builds on java 17 and 18 #239

Closed pjfanning closed 2 years ago

pjfanning commented 2 years ago
GedMarc commented 2 years ago

Okie my bad - took a while to find sorry

https://bugs.openjdk.java.net/browse/JDK-8196398

"wont-fix"

reason is because

"Etc/UTC" is categorized as "region-based ID" according to the ZoneId class description:

"The third type of ID are region-based IDs. A region-based ID must be of two or more characters, and not start with 'UTC', 'GMT', 'UT' '+' or '-'. Region-based IDs are defined by configuration, see ZoneRulesProvider. The configuration focuses on providing the lookup from the ID to the underlying ZoneRules."

This is different from "UTC". Even though TZ database regards UTC and Etc/UTC the same, (although very hypothetically) a ZoneRulesProvider can provide its own rules for Etc/UTC.

Use normalize() for tests (but possible keep? each value check cause that is great) as well as the string rep

Possible workaround for this is to compare ZoneIds with normalized() method.

Can also check based on the rules - but this is definitely per-os-config-defined

ZoneId z1 = ZoneId.of("Etc/UTC");
ZoneId z2 = ZoneId.of("UTC");
System.out.println(z1.equals(z2)); // false
System.out.println(z1.getRules().equals(z2.getRules())); // true
pjfanning commented 2 years ago

@GedMarc could you review the latest changes? Build is ok now.

GedMarc commented 2 years ago

@cowtowncoder there's a funny here, only reason JDK 8 passes, is because it is before update 152 on travis image / everyone using a later version of jdk8 would also experience the difference (tested)

The Temurin switch I do agree with, to make sure it runs with the latest builds, but as a test case, this could become flaky on backwards compatibility to jdk8 <= 151

Ref : https://bugs.openjdk.java.net/browse/JDK-8196398 The fact it was passing on JDK 8 is actually the bug

cowtowncoder commented 2 years ago

@GedMarc @pjfanning is this good to go? Happy to merge if so.

pjfanning commented 2 years ago

I'm happy with this