On Wed, 1 Dec 2021 18:02:05 GMT, Andrew Leonard <aleon...@openjdk.org> wrote:
>> Add a new --source-date <TIMESTAMP> (epoch seconds) option to jar and jmod
>> to allow specification of time to use for created/updated jar/jmod entries.
>> This then allows the ability to make the content deterministic.
>>
>> Signed-off-by: Andrew Leonard <anleo...@redhat.com>
>
> Andrew Leonard has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update src/jdk.jlink/share/classes/jdk/tools/jmod/JmodOutputStream.java
>
> Co-authored-by: Magnus Ihse Bursie <m...@icus.se>
Mainly just two minor comments: (1) there's a better date and time parser, and
(2) the current lower bound is a couple seconds off. I'll reply to your
question about (2) separately.
The `DateTimeFormatter.ISO_ZONED_DATE_TIME` zoned date and time parser works
better than the `ISO_DATE_TIME` local date and time parser when creating a
`ZonedDateTime` instance. It catches the following error slightely earlier and
provides a better error message.
Here's the current error using the local date and time parser:
$ jmod create --date 2011-12-03T10:15:30 ...
Error: --date 2011-12-03T10:15:30 is not a valid ISO 8601 date and time:
Text '2011-12-03T10:15:30' could not be parsed:
Unable to obtain ZonedDateTime from TemporalAccessor:
{},ISO resolved to 2011-12-03T10:15:30 of type java.time.format.Parsed
Here's the same error using the zoned date and time parser:
$ jmod create --date 2011-12-03T10:15:30 ...
Error: --date 2011-12-03T10:15:30 is not a valid ISO 8601 date and time:
Text '2021-11-29T09:36:06' could not be parsed at index 19
src/jdk.jartool/share/classes/sun/tools/jar/GNUStyleOptions.java line 199:
> 197: void process(Main jartool, String opt, String arg)
> throws BadArgs {
> 198: try {
> 199: jartool.date = ZonedDateTime.parse(arg,
> DateTimeFormatter.ISO_DATE_TIME)
The `ISO_ZONED_DATE_TIME` parser works better here.
src/jdk.jartool/share/classes/sun/tools/jar/GNUStyleOptions.java line 201:
> 199: jartool.date = ZonedDateTime.parse(arg,
> DateTimeFormatter.ISO_DATE_TIME)
> 200:
> .withZoneSameInstant(ZoneOffset.UTC).toLocalDateTime();
> 201: if (jartool.date.getYear() < 1980 ||
> jartool.date.getYear() > 2099) {
This `if` test actually compares down to the nanosecond, if present, but the
wrong nanosecond. How about something like the following?
... ZonedDateTime TIME_MIN = ZonedDateTime.parse("1980-01-01T00:00:02Z");
... ZonedDateTime TIME_MAX = ZonedDateTime.parse("2099-12-31T23:59:59Z");
...
var zoned = ZonedDateTime.parse(date,
DateTimeFormatter.ISO_ZONED_DATE_TIME);
if (zoned.isBefore(TIME_MIN) || zoned.isAfter(TIME_MAX)) {
....
src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 88:
> 86: date {0} is not a valid ISO 8601 date and time
> 87: error.date.out.of.range=\
> 88: date {0} is not within the valid year range 1980->2099
Maybe just define `date {0} is not within the valid range` and put the exact
interval in the *man* pages and documentation?
src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java line 1164:
> 1162: public LocalDateTime convert(String value) {
> 1163: try {
> 1164: LocalDateTime date = ZonedDateTime.parse(value,
> DateTimeFormatter.ISO_DATE_TIME)
The `ISO_ZONED_DATE_TIME` parser works better here.
src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java line 1166:
> 1164: LocalDateTime date = ZonedDateTime.parse(value,
> DateTimeFormatter.ISO_DATE_TIME)
> 1165:
> .withZoneSameInstant(ZoneOffset.UTC).toLocalDateTime();
> 1166: if (date.getYear() < 1980 || date.getYear() > 2099) {
See previous comment for this line in the `jar` tool.
src/jdk.jlink/share/classes/jdk/tools/jmod/resources/jmod.properties line 111:
> 109: err.no.moduleToHash=No hashes recorded: no module matching {0} found to
> record hashes
> 110: err.invalid.date=--date {0} is not a valid ISO 8601 date and time: {1}
> 111: err.date.out.of.range=--date {0} is out of the valid year range
> 1980->2099
As before, perhaps refer to the exact range in the documentation? It's really
an instant on the time line that is being compared and not the actual year
digits that were provided in the command-line option.
test/jdk/tools/jar/JarEntryTime.java line 180:
> 178: "2022-03-15T00:00:00+06:00",
> 179: "2038-11-26T06:06:06+00:00",
> 180: "2098-02-18T00:00:00-08:00"};
It would be great to test just inside the exact boundaries of the permitted
interval here.
test/jdk/tools/jar/JarEntryTime.java line 251:
> 249: // Negative Tests --date out of range source date
> 250: String[] badSourceDates = {"1976-06-24T01:02:03+00:00",
> 251: "2100-02-18T00:00:00-11:00"};
It would be great to test just outside the exact boundaries of the permitted
interval here.
-------------
Changes requested by jgneff (no project role).
PR: https://git.openjdk.java.net/jdk/pull/6481
