Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Imminent issue for Morocco users (tzdata2020a) #576

Closed
apaprocki opened this issue May 4, 2020 · 14 comments
Closed

Imminent issue for Morocco users (tzdata2020a) #576

apaprocki opened this issue May 4, 2020 · 14 comments
Assignees

Comments

@apaprocki
Copy link

ICU 66.1 only includes IANA time zone data tzdata2019c:
https://github.com/unicode-org/icu/blob/maint/maint-66/icu4c/source/data/misc/zoneinfo64.txt#L8

Upstream tzdata2020a was released to address the prior incorrect assumed temporary reversion of DST in Morocco due to Ramadan (eggert/tz@f9aacaf).

ICU merged unicode-org/icu@a951ab5 and released updated .res files for 2020a about 9 days ago. Unless the data files are manually updated and a new release is cut, users in Morocco and Intl API usage with zone Africa/Casablanca will see the time off by an hour between May 24th and May 31st.

@apaprocki
Copy link
Author

The tz-announce mailing list announces new releases, so is a good source to monitor: https://mm.icann.org/mailman/listinfo/tz-announce

Those then have to be packaged up and released as ICU binaries:
https://unicode-org.atlassian.net/browse/ICU-21094
unicode-org/icu-data#11
https://github.com/unicode-org/icu-data/tree/master/tzdata/icunew/2020a/44

There's usually a day (or few) lag between the two.

@srl295
Copy link
Member

srl295 commented May 11, 2020

ICU won't re-release 67 including this. For this we need… actually, we need to follow the process in your PR nodejs/node#30364 :)

but that won't help full-icu: for that we probably need some kind of side-load timezone process, which I mentioned in nodejs/node#32466

@apaprocki
Copy link
Author

actually, we need to follow the process in your PR

Yeah, I pinged @MylesBorins with this and let him know it would require that same patching approach to get it out in time.

but that won't help full-icu: for that we probably need some kind of side-load timezone process

FWIW I do that with our build of Node, using the out-of-band .res files on disk and the environment variable support to override the directory if necessary.

@codebytere
Copy link
Member

I have a patch locally which applies changes for this issue as specified in @apaprocki's doc PR - shall i go ahead and open that?

@srl295
Copy link
Member

srl295 commented May 12, 2020 via email

@srl295
Copy link
Member

srl295 commented May 19, 2020

PR LGTM.
Edit The pull request nodejs/node#33362 looks good. Since this is an imminent issue, I wanted to give watchers here a headsup.

@BethGriggs
Copy link
Member

We discussed this in the release meeting today (#580).

nodejs/node#33362 has now landed on master. The PR has also been cherry-picked on to v14.x-staging, ready to go out in the next v14.x release. Unfortunately, that is unlikely to happen before the 24th May (based on our current release schedule).

@srl295
Copy link
Member

srl295 commented May 21, 2020

@BethGriggs Thanks for the update. It's most critical right at that time of course, but can affect historical calculations also.

@richardlau
Copy link
Member

The updated timezone went out in 14.15.0. Node.js 12 hasn't been updated and is currently on timezone data 2019c (ICU 67). A manual backport is required as Node.js 12 is still built with small-icu by default while later release lines are built with full-icu.

@richardlau
Copy link
Member

Given that Node.js 12 is in maintenance, does anyone have an opinion on updating the timezone information vs leaving it as-is?

@albertyw
Copy link

We're running up to a repeat of this issue with Jordanian users (among other locales). tzdata 2021b (aka 2021a1) updates Jordan's DST taking effect on February 24, 2022 (https://mm.icann.org/pipermail/tz/2021-September/030760.html). It looks like the security releases yesterday didn't update node's ICU files on the LTS versions even though nodejs/node@de6399c and Node v17 are up-to-date.

Would it be possible to get Node v12, v14, and v16 released with updated versions of tzdata as soon as possible, else we would need to follow the env var override mentioned above.

@richardlau
Copy link
Member

We're running up to a repeat of this issue with Jordanian users (among other locales). tzdata 2021b (aka 2021a1) updates Jordan's DST taking effect on February 24, 2022 (https://mm.icann.org/pipermail/tz/2021-September/030760.html). It looks like the security releases yesterday didn't update node's ICU files on the LTS versions even though nodejs/node@de6399c and Node v17 are up-to-date.

Would it be possible to get Node v12, v14, and v16 released with updated versions of tzdata as soon as possible, else we would need to follow the env var override mentioned above.

We discussed this in yesterday's Release meeting and the plan is to try landing the ICU 70.1 update (containing the updated tz db) on Node.js 16 and 14. Node.js 12 needs a PR to update the timezone db (following https://github.com/nodejs/node/blob/master/doc/guides/maintaining-icu.md#time-zone-data) because Node.js 12 is built with small-icu so we cannot cherry-pick the ICU updates to it.

@albertyw
Copy link

Would nodejs/node#41443 be the PR needed for Node.js 12?

albertyw added a commit to albertyw/node-1 that referenced this issue Jan 14, 2022
This updates the time zone database up to 2021a4 for the node
v12.x-staging branch, following the documentation at
https://github.com/nodejs/node/blob/master/doc/guides/maintaining-icu.md#time-zone-data

This is essentially a backport of nodejs#40658.

Refs: nodejs#40658
Refs: nodejs/Release#576
@richardlau
Copy link
Member

Would nodejs/node#41443 be the PR needed for Node.js 12?

Yes, thanks! I'd missed this -- probably because it was closed (presumably accidentally) until you reopened it just now.

albertyw added a commit to albertyw/node-1 that referenced this issue Jan 16, 2022
This updates the time zone database up to 2021a4 for the node
v12.x-staging branch, following the documentation at
https://github.com/nodejs/node/blob/master/doc/guides/maintaining-icu.md#time-zone-data

This is essentially a backport of nodejs#40658.

Refs: nodejs#40658
Refs: nodejs/Release#576
ruyadorno pushed a commit to nodejs/node that referenced this issue Jan 26, 2022
This updates the time zone database up to 2021a4 for the node
v12.x-staging branch, following the documentation at
https://github.com/nodejs/node/blob/master/doc/guides/maintaining-icu.md#time-zone-data

This is essentially a backport of #40658.

Refs: #40658
Refs: nodejs/Release#576

PR-URL: #41443
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Ruy Adorno <ruyadorno@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants