Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: anba, Assigned: anba)

Tracking

Trunk
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Posted patch bug1310733.patch (obsolete) — Splinter Review
Patch generated with: 
  cd $MOZ/intl
  ./update-tzdata.sh -e $ICU56_BUILD_DIR/bin/icupkg 2016g

and then manually recorded the update as a "hg rename". 

I've used TZ=Europe/Istanbul to verify the new tzdata files are used.

Before: 
js> new Date(2016, 11, 1).toLocaleTimeString("en", {timeZoneName: "short"})
"11:00:00 PM GMT+2"

After:
js> new Date(2016, 11, 1).toLocaleTimeString("en", {timeZoneName: "short"})
"12:00:00 AM GMT+3"


I don't know if this change requires a clobber, but at least for local SpiderMonkey builds I had to build from scratch to pick up the changes.
Attachment #8801784 - Flags: review?(jwalden+bmo)
Comment on attachment 8801784 [details] [diff] [review]
bug1310733.patch

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

Manually recording as a rename shouldn't be necessary, IMO.  Either it should be part of the script -- which seems tricky, not least because it means the script would do very different things when updating versus running from scratch -- or we should just get rid of the changing portion.  Is there a reason intl/tzdata/source isn't an adequate directory to keep tzdata info in, so there isn't renaming each update?  (Followup is fine for this if you want.)
Attachment #8801784 - Flags: review?(jwalden+bmo) → review+
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #3)
> [...] or we should just get rid of the changing portion. 

Do you prefer a patch without renaming and just a simple "hg addremove"?

> Is there
> a reason intl/tzdata/source isn't an adequate directory to keep tzdata info
> in, so there isn't renaming each update?

It was useful for me to have multiple local tzdata directories for testing purposes. But I don't know if it's still useful to have this testing option.
From intl/tzdata/2015f to intl/tzdata/source?  That should be a manual rename to properly seed things, I think.

Having multiple directories for testing purposes seems not quite enough value, if we'd have to move anew every time.  I'd just do a build with patch not applied into one objdir, then a build with patch applied into another, if I wanted to compare.
I'll prepare two new patches:
1. Rename intl/tzdata/2015f to intl/tzdata/source, and change intl/update-tzdata.sh to export to intl/tzdata/source.
2. New svn export from ICU for tzdata2016g. This is also necessary to pick up the changes from last night: http://bugs.icu-project.org/trac/changeset/39460
Part 0:
- Rename intl/tzdata/2015f to intl/tzdata/source
- Update intl/update-tzdata.sh to export the time zone files to intl/tzdata/source, and automatically invoke "hg addremove" (similar to update-icu.sh)
Attachment #8801784 - Attachment is obsolete: true
Attachment #8802253 - Flags: review?(jwalden+bmo)
Part 1:
- Update to use tzdata2016g
Attachment #8802254 - Flags: review?(jwalden+bmo)
Attachment #8802253 - Flags: review?(jwalden+bmo) → review+
Attachment #8802254 - Flags: review?(jwalden+bmo) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/04d607a48165
Part 0: Rename tzdata svn directory to "source" instead of using version name. r=Waldo
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b2529d56816
Part 1: Update tzdata in ICU data files to 2016g. r=Waldo
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/04d607a48165
https://hg.mozilla.org/mozilla-central/rev/9b2529d56816
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.