Skip to content

Conversation

@GoeLin
Copy link
Member

@GoeLin GoeLin commented Nov 8, 2024

I backport this for parity with 21.0.6-oracle.
The documentation of the tz update says that it is not needed
necessarily, but a cleanup of various information. I think
we should go along here. It seems to bring a row of visible changes,
and it's good if this can be mapped to the same update everywhere.
Also, this will simplify later tz updates, which happen regularly.

CSR for update versions available.

This is a Backport of the jdk23u version, as the jdk24 version contains
changes breaking the Java standard. As I understand this only
affects Java 8. But Oracle avoided to bring these changes to 23,
so we should not do that in 21, either.

In detail, the 23u version omits backport of changes to
src/java.base/share/classes/java/time/ZoneId.java
and src/java.base/share/classes/sun/util/calendar/ZoneInfoFile.java.

In the tests changes to test/jdk/java/time/tck/java/time/TCKZoneId.java and
test/jdk/java/util/TimeZone/OldIDMappingTest.java are omited, and
test/jdk/sun/util/calendar/zi/TestZoneInfo310.java is modified differently. File
test/jdk/sun/util/calendar/zi/tzdata_jdk/tz2024b_overridden_zones
is added in 23. The change to test/jdk/sun/util/resources/TimeZone/Bug4848242.java differs, too.

Needed adaptions:

I had to resolve java/text/Format/DateFormat/TimeZoneNameTest.java.
But the new version breaks the test. I assume this is because 21 does
not have "8174269: Remove COMPAT
locale data provider from JDK".
The original version of the test passes.

test/jdk/sun/util/resources/TimeZone/Bug4848242.java is failing, too.
The original version works, reverted.

Further I reverted test/jdk/java/util/TimeZone/TimeZoneData/aliases.txt and .../displaynames.txt
as else java/util/TimeZone/Bug6329116.java (unmodified) starts failing.

See extra commits seperating out these edits.


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • JDK-8339637 needs maintainer approval
  • Change requires CSR request JDK-8342331 to be approved

Issues

  • JDK-8339637: (tz) Update Timezone Data to 2024b (Enhancement - P3 - Approved)
  • JDK-8342331: (tz) Update Timezone Data to 2024b (CSR)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk21u-dev.git pull/1143/head:pull/1143
$ git checkout pull/1143

Update a local copy of the PR:
$ git checkout pull/1143
$ git pull https://git.openjdk.org/jdk21u-dev.git pull/1143/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1143

View PR using the GUI difftool:
$ git pr show -t 1143

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk21u-dev/pull/1143.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Nov 8, 2024

👋 Welcome back goetz! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Nov 8, 2024

@GoeLin This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8339637: (tz) Update Timezone Data to 2024b

Reviewed-by: andrew

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 33 new commits pushed to the master branch:

  • cb9a6a2: 8333144: docker tests do not work when ubsan is configured
  • fd7b6e4: 8336640: Shenandoah: Parallel worker use in parallel_heap_region_iterate
  • 324e32f: 8321470: ThreadLocal.nextHashCode can be static final
  • 4b1365e: 8343923: GHA: Switch to Xcode 15 on MacOS AArch64 runners
  • 948710f: 8338389: [JFR] Long strings should be added to the string pool
  • b9c83b5: 8342823: Ubsan: ciEnv.cpp:1614:65: runtime error: member call on null pointer of type 'struct CompileTask'
  • a4927ca: 8338449: ubsan: division by zero in sharedRuntimeTrans.cpp
  • f36b3b4: 8336911: ZGC: Division by zero in heuristics after JDK-8332717
  • 4f6c506: 8344164: [s390x] ProblemList hotspot/jtreg/runtime/NMT/VirtualAllocCommitMerge.java
  • 38f654e: 8343724: [PPC64] Disallow OptoScheduling
  • ... and 23 more: https://git.openjdk.org/jdk21u-dev/compare/4a5c578bb092cde2c1cb40a451de6be63a9026f4...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot changed the title Backport 73b2341c670f98fb130c57f80eb1461226da1985 8339637: (tz) Update Timezone Data to 2024b Nov 8, 2024
@openjdk
Copy link

openjdk bot commented Nov 8, 2024

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added the backport Port of a pull request already in a different code base label Nov 8, 2024
@GoeLin GoeLin force-pushed the goetz_backport_8339637-23 branch from 6df4177 to 812a6bc Compare November 8, 2024 12:42
@gnu-andrew
Copy link
Member

Looking at this, but it will take a bit of time to work out what is going on with the tests.

The ZoneId change would affect every JDK that has java.time.ZoneId i.e. 8 and later. I think you might be confusing 8+ with just 8 alone. ZoneId defines EST, MST and HST as fixed offsets but the latest tzdata does not, instead making them links to continent/city-style timezones. In 24, they are changed to these new values in ZoneId but that requires a spec change that is not being backported to 23 and older.

The tz2024b_overridden_zones file brings back these fixed offsets. It does make me wonder if the 23u version of the tests were run against a tzdata with these fixed timezones back in play, as, for 21u, you seem to have ended up reverting the change which converts them from names to aliases. I'll try and dig into them further next week.

@GoeLin
Copy link
Member Author

GoeLin commented Nov 9, 2024

Hi @gnu-andrew, I just missed adding tz2024b_overridden_zones to the PR, but it was there when I ran all the tests in my local dir.
Our nighttests are green except for TestZoneInfo310.java which could not find above file.

Copy link
Member

@gnu-andrew gnu-andrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the java/text/Format, java/util/TimeZone, sun/util/calendar and sun/util/resources tests in four different scenarios:

  • Build of 23u with its own tests -- all passed
  • Build of 23u with the patches from this PR - failures from many tests, some untouched by this PR, due to changes post-21u
  • Build of 21u with the patches from this PR - passed
  • Build of 21u with the patches before the test reversion - three failures in java/text/Format/DateFormat/TimeZoneNameTest.java, java/util/TimeZone/Bug6329116.java and sun/util/resources/TimeZone/Bug4848242.java

In summary, this area has changed considerably due to changes in 22 & 23, notably JDK-8174269 and JDK-8317979. TimeZoneNameTest.java and Bug6329116.java are run with the COMPAT provider in 21u, which is removed by 8174269. The CLDR provider is using zone names from tzdata (JDK-8317979) while, I believe, COMPAT has its own set of names which are unaffected by the tzdata change.

I thus think most of the reversion is correct. There is one exception:

Link	Asia/Ulaanbaatar	Asia/Choibalsan

This is a change in the timezone data for 2024b; see "Asia/Choibalsan is now an alias for Asia/Ulaanbaatar rather than being a separate Zone with differing behavior before April 2008. This seems better given our wildly conflicting information about Mongolia's time zone history. (Thanks to Heitor David Pinto.)" from the tzdata release notes. So this change should be reinstated, but the ones related to the three-letter zone names are correct.

@GoeLin
Copy link
Member Author

GoeLin commented Nov 19, 2024

Hi @gnu-andrew
you mean I should add the link back to aliases.txt? I pushed another commit doing so.
Bug6329116 still passes.
Thanks for looking at this!

@GoeLin GoeLin marked this pull request as ready for review November 19, 2024 17:49
@openjdk openjdk bot added the rfr Pull request is ready for review label Nov 19, 2024
@mlbridge
Copy link

mlbridge bot commented Nov 19, 2024

Webrevs

Copy link
Member

@gnu-andrew gnu-andrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that one is an actual data change in this update, whereas the others are due to the three-letter zones being removed and don't seem to affect the COMPAT provider.

Change looks good to me now. Thanks for restoring that line.

@openjdk
Copy link

openjdk bot commented Nov 20, 2024

⚠️ @GoeLin This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

@openjdk openjdk bot added approval Requires approval; will be removed when approval is received ready Pull request is ready to be integrated and removed approval Requires approval; will be removed when approval is received labels Nov 20, 2024
@GoeLin
Copy link
Member Author

GoeLin commented Nov 21, 2024

/integrate

@openjdk
Copy link

openjdk bot commented Nov 21, 2024

Going to push as commit add7934.
Since your change was applied there have been 38 commits pushed to the master branch:

  • 322fc81: 8328619: sun/management/jmxremote/bootstrap/SSLConfigFilePermissionTest.java failed with BindException: Address already in use
  • 3aa59b3: 8341722: Fix some warnings as errors when building on Linux with toolchain clang
  • 2029e4e: 8337826: Improve logging in OCSPTimeout and SimpleOCSPResponder to help diagnose JDK-8309754
  • f9cd285: 8338550: Do libubsan1 installation in test container only if requested
  • 936b239: 8325525: Create jtreg test case for JDK-8325203
  • cb9a6a2: 8333144: docker tests do not work when ubsan is configured
  • fd7b6e4: 8336640: Shenandoah: Parallel worker use in parallel_heap_region_iterate
  • 324e32f: 8321470: ThreadLocal.nextHashCode can be static final
  • 4b1365e: 8343923: GHA: Switch to Xcode 15 on MacOS AArch64 runners
  • 948710f: 8338389: [JFR] Long strings should be added to the string pool
  • ... and 28 more: https://git.openjdk.org/jdk21u-dev/compare/4a5c578bb092cde2c1cb40a451de6be63a9026f4...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Nov 21, 2024
@openjdk openjdk bot closed this Nov 21, 2024
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Nov 21, 2024
@openjdk
Copy link

openjdk bot commented Nov 21, 2024

@GoeLin Pushed as commit add7934.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Port of a pull request already in a different code base integrated Pull request has been integrated

Development

Successfully merging this pull request may close these issues.

2 participants