On 07/01/2015 05:02 AM, Claes Redestad wrote:
Hi,
On 2015-06-30 23:11, Xueming Shen wrote:
Hi,
Please help review and comment on this rfe.
Issue: https://bugs.openjdk.java.net/browse/JDK-8075526
webrev: http://cr.openjdk.java.net/~sherman/8075526
I think this looks reasonable.
I think we could consolidate the LocalDateTime<->xdostime
conversions into ZipUtils along with and aligned with the code
to convert UTC instants (ZipUtils::dosToJavaTime/javaToDosTime),
which I've suggested should be converted into similar code
anyhow:
I would prefer to deal with JDK-8066644 issue separately. One of the concerns
of replacing the j.u.Date with the java.time classes was the possible startup
time regression that may be triggered by forcing the vm to load lots of
java.time
classes given all the jvm starts with loading bunch of jar files. As a matter of
fact, we spent the time and did the prototype when doing JSR310, it did show
startup regression back then... This might no longer be an issue if moving to
module system, but it'd be better to have some data first. On the other hand,
the proposed change here is for two new methods, no impact to any existing
apps.
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-February/031716.html
The get32S changes seems like an unrelated but welcome
optimization.
It's to fix the bug for negative date-time, which I missed when doing the unix
timestamp support. The 32-bit date-time need to be signed to suport date-time
before epoch. Well, you really should not have a zip file with last modified
time
before that though :-) Well in theory we still have the 2106 32-bit overflow
issue
for the unix timestamp, should find a solution later.
In above example, we still use the "system default timezone", however, it is
used
purely to output the zone name for the "zzz" (which the Date.toString() does),
not for conversion.
if the "zzz" is not required/needed, it can just be
java.time.format.DateTimeFormatter.ofPattern("EEE MMM dd HH:mm:ss
yyyy").format(e.getTimeLocal());
Since you're asking for opinions:
I think we should try not to propagate the problem we're working around here
(that values are printed/represented in a local time with no way of knowing what
timezone the time was local to). If we can print the local timezone at little
to no
cost, why not?
We actually might not be able to just change the jar tool to use the new
LocalDateTime
interface. It will be an "incompatible" change again :-) As in the past there
were regression
complains that there are tools "over there" that have dependency on the output
format
of the jar tool. Like my example, even with "zzz", the j.t.f.DTF will output
the zonename
for local here now as "pt" but the Date.toString() go with "PDT", because
without tz lookup
(with either a ldt or an instant) you don't know if it's PDT or PST. Well
someone says it really
does not matter:-)
Thanks for the review.
-Sherman
