Re: RFR: 8334048: -Xbootclasspath can not read some ZIP64 zip files [v4]
On Tue, 10 Sep 2024 05:46:46 GMT, Jaikiran Pai wrote: >> I pushed the review suggestions. There were two GitHub Actions failures on >> `macos-aarch64`, but they look to be occurrences of this existing bug: >> https://bugs.openjdk.org/browse/JDK-8247940. > > Hello @fitzsim, I plan to take a look at this change and also consult with > others familiar with the bootclasspath area as well as jar/zip area. I am > just noting this now to let you know that the PR hasn't been neglected and I > think it will take a while for this to be reviewed given the nature and area > of this change. > Sounds good @jaikiran. > > I can provide some background about this pull request; it was the result of > discussions on a related jdk8u-dev pull request > ([openjdk/jdk8u-dev#452](https://github.com/openjdk/jdk8u-dev/pull/452)). > > Red Hat has been shipping the `zip_util.c` change proposed here, authored by > @gnu-andrew, as a patch to our Red Hat Enterprise Linux `java-1.8.0-openjdk` > packages since mid-2020. In that release all `ZIP` files the `JDK` uses are > processed by `zip_util.c` unless someone is using the demo `Java` > implementation. We have had no field reports of issues with the patch. > > @gnu-andrew and I would like to upstream `ZIP64` support to `jdk8u`, but the > `zip_util.c` patch only exists within Red Hat's package repository, not > anywhere upstream from where it could be backported. > > When I checked for the most recent branch that still shipped `zip_util.c`, I > discovered that even mainline still ships it, albeit now only for use in > `-Xbootclasspath` handling. So I split the `zip_util.c` patch out and added > some tests to demonstrate the bug itself and that there should be no > regressions, and filed this pull request. > > If this is accepted into mainline and gets some broader testing with no > issue, then I would like to backport it to 21, 17, 11, and 8. Just to add to this, while the `zip_util.c` changes are unique to our 8u patch, they are a conversion to C of the equivalent Java changes made in [JDK-8186464](https://bugs.openjdk.org/browse/JDK-8186464), with the intention to backport that change to 8u without the risk of [JDK-8145260](https://bugs.openjdk.org/browse/JDK-8145260), which introduces the Java-based implementation. So if there is a risk in this patch, it is likely a risk of a mistake in the translation to C rather than the general gist of the changes, which are present in the JDK's Java zip code. - PR Comment: https://git.openjdk.org/jdk/pull/19678#issuecomment-2468614066
Re: RFR: 8334048: -Xbootclasspath can not read some ZIP64 zip files [v3]
On Mon, 2 Sep 2024 16:02:31 GMT, Andrew John Hughes wrote: >> fitzsim has updated the pull request incrementally with one additional >> commit since the last revision: >> >> BootClassPathZipFileTest: Switch to createClassBytes, createZip static >> functions > > I can't review, as I ported the native code changes. On that note, can you > make sure I'm credited as co-author using > [/contributor](https://wiki.openjdk.org/display/SKARA/Pull+Request+Commands#PullRequestCommands-/contributor) > please? > > i.e. /contributor add @gnu-andrew > > You may want to also comment on > [JDK-8185896](https://bugs.openjdk.org/browse/JDK-8185896) which is about > adding Zip64 tests. > Hi @gnu-andrew, I have corrected your contributor status, thank you for the > reminder. > > I do not have access to comment on > https://bugs.openjdk.org/browse/JDK-8185896. Can you please add a comment > from me that says: > > "I have added to #19678 two ZIP64 test cases: one for ZIP64 extensions added > implicitly due to one of the CEN fields being too large to represent, and one > for ZIP64 extensions that have been added explicitly even though no CEN field > required them. > > It is not complete coverage -- the "implicit" test only tests the field for > the total number of entries. But the coverage they provide may be enough to > warrant closing JDK-8185896. > > The test cases are fast and portable back to OpenJDK 8, and do not require > significant disk space, so they can be run unconditionally as part of the > default `jtreg` test set." Done. Thanks for adding the attribution. - PR Comment: https://git.openjdk.org/jdk/pull/19678#issuecomment-2327687252
Re: RFR: 8334048: -Xbootclasspath can not read some ZIP64 zip files [v3]
On Wed, 31 Jul 2024 21:54:15 GMT, fitzsim wrote: >> 8334048: -Xbootclasspath can not read some ZIP64 zip files > > fitzsim has updated the pull request incrementally with one additional commit > since the last revision: > > BootClassPathZipFileTest: Switch to createClassBytes, createZip static > functions I can't review, as I ported the native code changes. On that note, can you make sure I'm credited as co-author using [/contributor](https://wiki.openjdk.org/display/SKARA/Pull+Request+Commands#PullRequestCommands-/contributor) please? i.e. /contributor add @gnu-andrew You may want to also comment on [JDK-8185896](https://bugs.openjdk.org/browse/JDK-8185896) which is about adding Zip64 tests. - PR Comment: https://git.openjdk.org/jdk/pull/19678#issuecomment-2325035829
Re: RFR: 8334441: Mark tests in jdk_security_infra group as manual [v2]
On Fri, 21 Jun 2024 16:11:34 GMT, Rajan Halade wrote: >> Updated all the tests that depend on external infrastructure services as >> manual. These tests may fail with external reasons, for instance - change in >> CA test portal, certificate status updates, or network issues. > > Rajan Halade has updated the pull request incrementally with one additional > commit since the last revision: > > fix typos Is there still going to be some monitoring of these tests? Making it harder to run these tests potentially means that genuine failures can go unnoticed and JDK certificates are not updated when needed. - PR Comment: https://git.openjdk.org/jdk/pull/19814#issuecomment-2189601735
Re: RFR: 8305113: (tz) Update Timezone Data to 2023c [v3]
On Mon, 3 Apr 2023 23:40:05 GMT, Yoshiki Sato wrote: >> Pleases review this PR. >> The PR includes the following changes. >> - tzdata 2023c >> - Hack code to deal with Cairo's DST end, which is same as done in >> 2014g([JDK-8049343](https://bugs.openjdk.org/browse/JDK-8049343)). To work >> around the the limitation of JSR 310 compiler, where the time 24:00 is >> recognized as 0:00 in the previous day. > > Yoshiki Sato has updated the pull request incrementally with two additional > commits since the last revision: > > - Revert formatted code in ZoneInfoFile.java > - fixing typo again Looks good to me. Interesting that `TimeZoneNames.java` already has the `MSK` change for the other timezone that was changed, `Europe/Volgograd` - Marked as reviewed by andrew (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13255#pullrequestreview-1371346092
Integrated: 8297804: (tz) Update Timezone Data to 2022g
On Wed, 30 Nov 2022 18:07:07 GMT, Andrew John Hughes wrote: > Update to the latest tzdata, 2022g. > > Primary changes: > * `America/Ojinaga` (CST) is split, creating `America/Ciudad_Juarez` (MST/MDT) > * `America/Pangnirtung` becomes a link to `America/Iqaluit` > * `America/Ojinaga` gains DST (CDT) > > See bug for the full details. > > There will likely also be CLDR changes > ([CLDR-16181](https://unicode-org.atlassian.net/browse/CLDR-16181)) that are > not yet included in this change. We can either wait for these and include > them in this patch, or do a separate change like > [JDK-8296715](https://bugs.openjdk.org/browse/JDK-8296715) > > Tests in `java/util/TimeZone`, `java/time/test` and `sun/text/resources` all > pass. This pull request has now been integrated. Changeset: ce896731 Author:Andrew John Hughes URL: https://git.openjdk.org/jdk/commit/ce896731d38866c2bf99cd49525062e150d94160 Stats: 240 lines in 26 files changed: 144 ins; 57 del; 39 mod 8297804: (tz) Update Timezone Data to 2022g Reviewed-by: naoto - PR: https://git.openjdk.org/jdk/pull/11438
Re: RFR: 8297804: (tz) Update Timezone Data to 2022g [v2]
On Mon, 5 Dec 2022 14:38:51 GMT, Andrew John Hughes wrote: >> Update to the latest tzdata, 2022g. >> >> Primary changes: >> * `America/Ojinaga` (CST) is split, creating `America/Ciudad_Juarez` >> (MST/MDT) >> * `America/Pangnirtung` becomes a link to `America/Iqaluit` >> * `America/Ojinaga` gains DST (CDT) >> >> See bug for the full details. >> >> There will likely also be CLDR changes >> ([CLDR-16181](https://unicode-org.atlassian.net/browse/CLDR-16181)) that are >> not yet included in this change. We can either wait for these and include >> them in this patch, or do a separate change like >> [JDK-8296715](https://bugs.openjdk.org/browse/JDK-8296715) >> >> Tests in `java/util/TimeZone`, `java/time/test` and `sun/text/resources` all >> pass. > > Andrew John Hughes has updated the pull request incrementally with two > additional commits since the last revision: > > - Add CLDR changes from > https://github.com/unicode-org/cldr/commit/0bd22412b35b6e7037a39c3ad6a4dc49c699439b > - Update tzdata version Thanks. - PR: https://git.openjdk.org/jdk/pull/11438
Re: RFR: 8297804: (tz) Update Timezone Data to 2022g [v2]
On Mon, 5 Dec 2022 20:37:31 GMT, Naoto Sato wrote: > Have you run tests under `sun.util.calendar`? Tests like > `TestZoneInfo310.java` sometimes fail with tzdata updates. Yes ~~~ Passed: sun/util/calendar/zi/Beyond2037.java Passed: sun/util/calendar/zi/TestZoneInfo310.java Passed: sun/util/calendar/Bug6653944.java Passed: sun/util/calendar/Bug8176160.java Passed: sun/util/calendar/CalendarSystemDeadLockTest.java ~~~ `sun/util` and `java/util` are also in `jdk_util_other` which is covered by `tier1_part2` in the PR checks: https://github.com/gnu-andrew/jdk/actions/runs/3621042294/jobs/6104922328#step:9:5925 Only `java.time` and `sun.text` are not covered by the PR (`tier2_part2`) - PR: https://git.openjdk.org/jdk/pull/11438
Re: RFR: 8297804: (tz) Update Timezone Data to 2022g
On Sat, 3 Dec 2022 00:21:21 GMT, Naoto Sato wrote: > Hi Andrew, Thanks for taking on this. I think you can cherry-pick the > relevant [CLDR > changes](https://github.com/unicode-org/cldr/commit/0bd22412b35b6e7037a39c3ad6a4dc49c699439b) > (under `common` directory) into this PR so that backporting would be easier. Good to see CLDR included them in good time. Now in this patch. - PR: https://git.openjdk.org/jdk/pull/11438
Re: RFR: 8297804: (tz) Update Timezone Data to 2022g
On Fri, 2 Dec 2022 01:15:49 GMT, Yoshiki Sato wrote: > The following files need to be updated with the timezone data version. > src/java.base/share/data/tzdata/VERSION > test/jdk/java/util/TimeZone/TimeZoneData/VERSION Good catch. I copied over the files from the tzdb bundle, but it seems they have `version` while we have `VERSION`. Fixed now. - PR: https://git.openjdk.org/jdk/pull/11438
Re: RFR: 8297804: (tz) Update Timezone Data to 2022g [v2]
> Update to the latest tzdata, 2022g. > > Primary changes: > * `America/Ojinaga` (CST) is split, creating `America/Ciudad_Juarez` (MST/MDT) > * `America/Pangnirtung` becomes a link to `America/Iqaluit` > * `America/Ojinaga` gains DST (CDT) > > See bug for the full details. > > There will likely also be CLDR changes > ([CLDR-16181](https://unicode-org.atlassian.net/browse/CLDR-16181)) that are > not yet included in this change. We can either wait for these and include > them in this patch, or do a separate change like > [JDK-8296715](https://bugs.openjdk.org/browse/JDK-8296715) > > Tests in `java/util/TimeZone`, `java/time/test` and `sun/text/resources` all > pass. Andrew John Hughes has updated the pull request incrementally with two additional commits since the last revision: - Add CLDR changes from https://github.com/unicode-org/cldr/commit/0bd22412b35b6e7037a39c3ad6a4dc49c699439b - Update tzdata version - Changes: - all: https://git.openjdk.org/jdk/pull/11438/files - new: https://git.openjdk.org/jdk/pull/11438/files/27bf9277..82d3453b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11438&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11438&range=00-01 Stats: 23 lines in 5 files changed: 19 ins; 1 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/11438.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11438/head:pull/11438 PR: https://git.openjdk.org/jdk/pull/11438
RFR: 8297804: (tz) Update Timezone Data to 2022g
Update to the latest tzdata, 2022g. Primary changes: * `America/Ojinaga` (CST) is split, creating `America/Ciudad_Juarez` (MST/MDT) * `America/Pangnirtung` becomes a link to `America/Iqaluit` * `America/Ojinaga` gains DST (CDT) See bug for the full details. There will likely also be CLDR changes ([CLDR-16181](https://unicode-org.atlassian.net/browse/CLDR-16181)) that are not yet included in this change. We can either wait for these and include them in this patch, or do a separate change like [JDK-8296715](https://bugs.openjdk.org/browse/JDK-8296715) Tests in `java/util/TimeZone`, `java/time/test` and `sun/text/resources` all pass. - Commit messages: - 8297804: (tz) Update Timezone Data to 2022g Changes: https://git.openjdk.org/jdk/pull/11438/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11438&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8297804 Stats: 217 lines in 21 files changed: 125 ins; 56 del; 36 mod Patch: https://git.openjdk.org/jdk/pull/11438.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11438/head:pull/11438 PR: https://git.openjdk.org/jdk/pull/11438
Re: RFR: 8292579: (tz) Update Timezone Data to 2022c
On Thu, 25 Aug 2022 02:26:19 GMT, Yoshiki Sato wrote: > Please review this PR. The PR adopts the official tzdata2022c as it is. > It means the pre-1970s data merged in tzdata2022c doesn't exist. > All tests have been passed with no failures. I've opened https://bugs.openjdk.org/browse/JDK-8293834 for the CLDR backport and will open a PR tomorrow for 19u. - PR: https://git.openjdk.org/jdk/pull/10012
Re: RFR: 8292579: (tz) Update Timezone Data to 2022c
On Thu, 25 Aug 2022 02:26:19 GMT, Yoshiki Sato wrote:
> Please review this PR. The PR adopts the official tzdata2022c as it is.
> It means the pre-1970s data merged in tzdata2022c doesn't exist.
> All tests have been passed with no failures.
Interesting, I remember changes like this causing failures in the past. Maybe
they were introducing completely new zones. It looks good for the old
`java.util` API as well:
~~~
$ ~/builder/trunk/images/jdk/bin/jshell -R-Djava.locale.providers=COMPAT JAVASE
| Welcome to JShell -- Version 20-internal
| For an introduction type: /help intro
jshell> ZoneId.of("Europe/Kyiv").getDisplayName(TextStyle.FULL, Locale.US)
$178 ==> "Eastern European Time"
jshell> TimeZone.getTimeZone("Europe/Kyiv").getDisplayName(false,
TimeZone.LONG, Locale.US)
$179 ==> "Eastern European Time"
~~~
and similar for 11u & 17u with an updated tzdb. I guess we just need the CLDR
change in a backport then.
-
PR: https://git.openjdk.org/jdk/pull/10012
Re: RFR: 8292579: (tz) Update Timezone Data to 2022c
On Thu, 25 Aug 2022 02:26:19 GMT, Yoshiki Sato wrote: > Please review this PR. The PR adopts the official tzdata2022c as it is. > It means the pre-1970s data merged in tzdata2022c doesn't exist. > All tests have been passed with no failures. That is something we've patched locally e.g. https://src.fedoraproject.org/rpms/java-11-openjdk/pull-request/163 and we have a test which fails without and passes with, though it does use `sun.util.resources` directly. - PR: https://git.openjdk.org/jdk/pull/10012
Re: RFR: 8292579: (tz) Update Timezone Data to 2022c
On Thu, 25 Aug 2022 02:26:19 GMT, Yoshiki Sato wrote: > Please review this PR. The PR adopts the official tzdata2022c as it is. > It means the pre-1970s data merged in tzdata2022c doesn't exist. > All tests have been passed with no failures. Should this not have included an update to the resource bundles - as with https://github.com/openjdk/jdk/commit/ad3be04d2ac84836e393d696ff03ddfe72779094 - so that `Europe/Kyiv` is recognised? - PR: https://git.openjdk.org/jdk/pull/10012
