Closed Bug 1502530 Opened 11 months ago Closed 11 months ago

Update to tzdata2018g

Categories

(Core :: JavaScript: Internationalization API, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 --- fixed
firefox63 --- wontfix
firefox64 --- fixed
firefox65 --- fixed

People

(Reporter: anba, Assigned: anba)

Details

Attachments

(5 files, 2 obsolete files)

ICU's tzdata updates for tzdata 2018f aren't yet released (bug 1499031), but soon we'll probably get a 2018g release anyway because of <https://mm.icann.org/pipermail/tz/2018-October/027106.html>.
Release notes: https://mm.icann.org/pipermail/tz-announce/2018-October/000052.html

Now waiting for the updated ICU tz-data.
Modifies the update script to use the new GitHub location.
Assignee: nobody → andrebargull
Status: NEW → ASSIGNED
Attachment #9022665 - Flags: review?(jwalden+bmo)
The actual update to 2018g.
Attachment #9022666 - Flags: review?(jwalden+bmo)
Comment on attachment 9022665 [details] [diff] [review]
bug1502530-part1-update-script.patch

Review of attachment 9022665 [details] [diff] [review]:
-----------------------------------------------------------------

::: intl/update-tzdata.sh
@@ +55,5 @@
>  icu_tzdata_version=`grep --only-matching --perl-regexp --regexp="tz version:\s+\K.*$" "${icu_dir}"/source/data/misc/zoneinfo64.txt`
>  local_tzdata_version=
>  if [ -f "${tzdata_dir}"/SVN-INFO ]; then
>    local_tzdata_version=`grep --only-matching --perl-regexp --regexp="^URL: .*tzdata/icunew/\K[0-9a-z]+" "${tzdata_dir}"/SVN-INFO`
>  fi

The SVN-INFO bits here aren't going to be removed?  Or do they get removed in the next patch or something?  Not sure why both SVN-INFO and GIT-INFO should be dealt with.

@@ +130,5 @@
>    # Remove intl/tzdata/source, then replace it with a clean export.
>    rm -r "${tzdata_dir}"/source
> +  git clone --depth 1 "${tzdata_url}" "${tzdata_dir}"/source
> +  pushd "${tzdata_dir}"/source
> +  git filter-branch --prune-empty --subdirectory-filter tzdata/icunew/${tzdata_version}/44 HEAD

I think I'd probably prefer no pushd/popd here, instead using git's -C option and using "${tzdata_dir}" for the other paths being referred to here.  IMO.  Changing of working-directory state feels unpleasant.
Attachment #9022665 - Flags: review?(jwalden+bmo) → review+
Attachment #9022666 - Flags: review?(jwalden+bmo) → review+
Updated part 1 per review comments and additionally changed it to store the version information into a separate file instead of appending it to GIT-INFO. Carrying r+.
Attachment #9022665 - Attachment is obsolete: true
Attachment #9022905 - Flags: review+
Update part 2 to reflect the changed version information approach from part 1. Carrying r+.
Attachment #9022666 - Attachment is obsolete: true
Attachment #9022906 - Flags: review+
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/938fbccd6386
Part 1: Update tzdata updater to use new location. r=Waldo
https://hg.mozilla.org/integration/mozilla-inbound/rev/40e2f539938d
Part 2: Update tzdata in ICU data files to 2018g. r=Waldo
Keywords: checkin-needed
Forgot to regenerate the SpiderMonkey-internal time zone files when uploading the updated part 2. Setting to r+ because they were already reviewed in the previous part 2 patch.
Attachment #9022927 - Flags: review+
Another check-in is needed for "bug1502530-part3-spidermonkey-tz-files.patch". Sorry for the hassle!
Keywords: checkin-needed
Backport for part 2 (directly includes the missing part 3) for Beta.

Part 1 applies cleanly on Beta.
Attachment #9022933 - Flags: review?(jwalden+bmo)
Backport for part 2 (directly includes the missing part 3) for ESR60.

Part 1 applies cleanly on ESR60.
Attachment #9022934 - Flags: review?(jwalden+bmo)
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea56e5953736
Part 3: Update SpiderMonkey time zone files. r=Waldo
Keywords: checkin-needed
(In reply to André Bargull [:anba] from comment #10)
> Another check-in is needed for
> "bug1502530-part3-spidermonkey-tz-files.patch". Sorry for the hassle!

No problem, patch landed above.
Attachment #9022933 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 9022934 [details] [diff] [review]
esr60-bug1502530-part2-update-data.patch

Review of attachment 9022934 [details] [diff] [review]:
-----------------------------------------------------------------

It belatedly occurs to me that keeping up to date is going to require backporting update script updates, where the update is necessary for the script to keep working.  This is not the end of the world.  Still, unfortunate.
Attachment #9022934 - Flags: review?(jwalden+bmo) → review+
The update script doesn't require any changes for the backports, therefore I haven't requested a separate review for them.
Comment on attachment 9022933 [details] [diff] [review]
beta-bug1502530-part2-update-data.patch

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: N/A

User impact if declined: Wrong time zone data when calling standard Date and Intl.DateTimeFormat methods.

Based on the release notes for tzdata 2018f <http://mm.icann.org/pipermail/tz-announce/2018-October/000051.html> and 2018g <http://mm.icann.org/pipermail/tz-announce/2018-October/000052.html>, the update is of special interest for users in:
- Volgograd (time zone changed from +03 to +04 on 2018-10-28)
- Fiji (DST end changed from 2019-01-20 to 2019-01-13)
- Chile (DST start and end dates changed starting 2019-04-06)
- Morocco (time zone changed to permanent +01 on 2018-10-27)

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): It only updates time zone data files.

String changes made/needed: No.
Attachment #9022933 - Flags: approval-mozilla-beta?
Comment on attachment 9022934 [details] [diff] [review]
esr60-bug1502530-part2-update-data.patch

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: See comment #18

User impact if declined: See comment #18

Fix Landed on Version: 64/65

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): See comment #18

String or UUID changes made by this patch: None
Attachment #9022934 - Flags: approval-mozilla-esr60?
Comment on attachment 9022905 [details] [diff] [review]
bug1502530-part1-update-script.patch

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: N/A

User impact if declined: The changes for this Python script are only necessary for users who build Firefox on their own and like to have reproducible builds. The script doesn't contain any code which is run automatically, but instead needs to be run manually by Firefox developers.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The Python script is only run manually by Firefox developers when updating the tzdata files.

String changes made/needed: No.

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: See above.

User impact if declined: See above.

Fix Landed on Version: 65

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): See above.

String or UUID changes made by this patch: None.
Attachment #9022905 - Flags: approval-mozilla-esr60?
Attachment #9022905 - Flags: approval-mozilla-beta?
"beta-bug1502530-part2-update-data.patch" and "esr60-bug1502530-part2-update-data.patch" contain the changes from "bug1502530-part2-update-data.patch" *and* "bug1502530-part3-spidermonkey-tz-files.patch", therefore we don't need to have a separate backport for "bug1502530-part3-spidermonkey-tz-files.patch" for Beta/ESR.
Comment on attachment 9022905 [details] [diff] [review]
bug1502530-part1-update-script.patch

tzdata update for 64.0 rc1.
Attachment #9022905 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9022933 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 9022905 [details] [diff] [review]
bug1502530-part1-update-script.patch

OK for ESR uplift.
Attachment #9022905 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Attachment #9022934 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
This had problems landing on ESR and from talking with Aryx it sounds like this may need some discussion first. Can this wait till ESR 60.5.0?
Flags: needinfo?(jwalden+bmo)
Sorry about this, one of the patches applied during the first landing attempt shouldn't have been used by me. This has landed on esr60 now.
Flags: needinfo?(jwalden+bmo)
(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #26)

To be clear, this missed the build of 60.4.0esr. It will be in the next release we ship from that branch, either 60.4.1esr or 60.5.0esr.
You need to log in before you can comment on or make changes to this bug.