1257736 - Fix timezone update script so that it runs

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Geoff Lankow (:darktrojan)]

The changes to the update script to save the previous data in the tree don't actually work. I'm going to fix it and update the zones.

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: 1257736a-1.diff

As well as fixing the bug I've cleaned up some formatting nits my linter was complaining about.

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: 1257736b-1.diff

Update to 2016b. This diff has whitespace changes ignored due to previous.json having Windows line-endings that I've removed.

Comment 3

[[:MakeMyDay]]

Patch 1257736b-1.diff should be moved to bug 1255878 and then applied to all branches including both esr branches.

We should leave this bug for patch 1257736a-1.diff as that needs only to go to cc and ca.

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8731991 [details] [diff] [review]
1257736b-1.diff

The version says 2.2015g, but the new timezones are added with 2.2016b. Can you fix this and then maybe move that to the bug mentioned?

Comment 5

[Geoff Lankow (:darktrojan)]

(In reply to Philipp Kewisch [:Fallen] from comment #4)
> Comment on attachment 8731991 [details] [diff] [review]
> 1257736b-1.diff
> 
> The version says 2.2015g, but the new timezones are added with 2.2016b. Can
> you fix this and then maybe move that to the bug mentioned?

Sure you're not reading the version number of previous.json?

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8731991 [details] [diff] [review]
1257736b-1.diff

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

I certainly am, sorry about that!

Comment 7

[Geoff Lankow (:darktrojan)]

Attachment: 1257736c-1.diff

Might as well do this while waiting for the tree to open.

Comment 8

[Geoff Lankow (:darktrojan)]

I've pushed the first two, since the tree is open.

https://hg.mozilla.org/comm-central/rev/dbc08726f871
https://hg.mozilla.org/comm-central/rev/7459c860a7db

Comment 9

[[:MakeMyDay]]

Comment on attachment 8734995 [details] [diff] [review]
1257736c-1.diff

That timezone changes look odd. If you look for example at the change for Baku, the DST is removed. That means now an offset will be applied to all previous events in the previous dst period of previous years by one hour (the same will be true when adding new DST - just the other way around) - so imho the existing dst definition has to stay but needs to be extended by an dtend some time around last October.

With other words, if the gdata provider test fails after update, it's not the test that needs to be fixed here but the timezone definition.

This seems to be a problem not just for the changes made with this patch but to all nearly definitions in zones.json.

Comment 10

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8734995 [details] [diff] [review]
1257736c-1.diff

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

I'll inquire about how other clients handle the timezone changes when I am at calconnect. r/a+ for this patch on all branches including both ESR's, sorry for the delay.

Comment 11

[[:MakeMyDay]]

Thanks. Patch B still needs the same approvals.

Comment 12

[Stefan Sitter [:ssitter]]

Comment on attachment 8731991 [details] [diff] [review]
1257736b-1.diff

This patch cannot land on aurora, beta, and release as is because it adds new strings and therefore has L10N impact.

Maybe a new patch could be created that updates only the existing timezone definitions but doesn't add new ones on the release branch?

Comment 13

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

As per http://mxr.mozilla.org/comm-central/source/calendar/locales/filter.py#11 I believe this shouldn't be a problem, since the strings do not cause errors. Now that we do l10n merges, the english strings will be merged in.

While this isn't ideal, it is better than not doing timezone updates on an old branch. I can reconfirm with Pike though to make sure this won't cause any issues.

Comment 14

[Geoff Lankow (:darktrojan)]

Part 3:
https://hg.mozilla.org/comm-central/rev/9429854fd82f

Comment 15

[[:MakeMyDay]]

The patches B and C still need an uplift to all branches including both ESR branches - at best before the merge after the upcoming weekend. When landing on beta, make sure to land on default. Geoff, can you take care, too?

Comment 16

[[:MakeMyDay]]

Attachment: 1232209etal-esr38-v1.diff

To backport the timezone updates (patch B and C) to esr38, further backports from bug 1180522, bug 1210458, bug 1232209, bug 1232216 and bug 1237085 are needed. This patch cummulates all relevant parts of them. This is intended to land on esr38 only.

The change from 1237085 to calendar/providers/gdata/modules/timezoneMap.jsm is not included as gdata provider bases on cc, which has the respective change already.

Comment 17

[[:MakeMyDay]]

Backports for the tz updates are landed up to esr45 (referenced in bug 1255878). Only esr38 is missing.

Comment 18

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8742109 [details] [diff] [review]
1232209etal-esr38-v1.diff

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

Doing an AMO release for Lightning 4.0 after I have uploaded 4.7 will probably be tricky/impossible, but I'm ok with this for the last ESR38 distribution release.

Comment 19

[[:MakeMyDay]]

TZ updates backported to esr38 (referenced in bug 1255878).