643199 - Correctly remove outdated L10n files on update

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Robert Kaiser]

As noted in bug 633394, we leave around old L10n files on update, which was made very visible with the omnijar switch - the reason is that we don't generate a new removed-files with the correct locale code when doing the repacks.

The general code for this will be done on the toolkit side (bug 592574), we just need a smaller patch to "activate" it and make sure our removed-files gets the correct variables when regenerating it from locales/Makefile.

Comment 1

[Robert Kaiser]

Attachment: v1: add needed defines

This patch does the SeaMonkey-specific part, but the l10n.mk part is of course needed for it (if that doesn't get accepted, we can do all on our side, but doing it there is better).

Comment 2

[Justin Wood (:Callek)]

Comment on attachment 520488 [details] [diff] [review]
v1: add needed defines

r+ conditional on if the m-c bug lands. [I _hope_ it is accepted into m-2.0 there]

Comment 3

[Robert Kaiser]

Putting this on the radar for final. Looks like nothing is moving in m-c there, so we might need to go with the "dirty" solution of putting in all possible locale files explicitely.

Comment 4

[Edmund Wong (:ewong)]

Attachment: Correctly remove outdated L10n files on update.

Comment 5

[Edmund Wong (:ewong)]

Attachment: Correctly remove outdated L10n files on update.

Comment 6

[Justin Wood (:Callek)]

Comment on attachment 530482 [details] [diff] [review]
Correctly remove outdated L10n files on update.

>-chrome/@AB_CD@.manifest
>+chrome/ca.manifest
>+chrome/de.manifest
>+chrome/es-ES.manifest

need en-US here as well

> #ifdef MOZ_OMNIJAR
>-  chrome/@AB_CD@.jar
>+  chrome/ca.jar
>+  chrome/de.jar
>+  chrome/es-ES.jar

Same here

>+  extensions/[e-mail]/chrome/venkman.jar
>+  extensions/[e-mail]/chrome.manifest

I'm going to take this section as an example of what I mean.... I'd like ALL the chatzilla entries above all the venkman entries (like you do have here), BUT I want it also grouped by locale above file.

So you'd have 

extensions/[e-mail]/chrome/venkman.jar
extensions/[e-mail]/chrome.manifest
....
extensions/[e-mail]/chrome/venkman.jar
extensions/[e-mail]/chrome.manifest
...

make sense?

Comment 7

[Justin Wood (:Callek)]

(In reply to comment #6)
> >+  extensions/[e-mail]/chrome/venkman.jar

...wait huh, this was broken before. can you make this chatzilla.jar [instead of venkman.jar] while you're here please?

Comment 8

[Edmund Wong (:ewong)]

Attachment: Correctly remove outdated L10n files on update. (v3)

Comment 9

[Edmund Wong (:ewong)]

Attachment: Correctly remove outdated L10n files on update. (v4)

Comment 10

[Justin Wood (:Callek)]

Comment on attachment 530517 [details] [diff] [review]
Correctly remove outdated L10n files on update. (v4)

>+  extensions/[e-mail]/chrome/chatzilla.jar
>+  extensions/[e-mail]/chrome.manifest
>+  extensions/[e-mail]/install.js
>+  extensions/[e-mail]/install.rdf
>+  extensions/[e-mail]

We don't need en-US for chatzilla here (its a langpack)

>+  extensions/[e-mail]/chrome/venkman.jar
>+  extensions/[e-mail]/chrome.manifest
>+  extensions/[e-mail]/install.rdf
>+  extensions/[e-mail]

Same here.


Also chatzilla and venkman have more locales listed than needed, but it is NOT NECESSARY to fix for this bug (we can do it in a followup) as them being listed is not a problem.

The chatzilla locale list: http://mxr.mozilla.org/comm-central/source/mozilla/extensions/irc/locales/all-locales
The Venkman Locale List: http://mxr.mozilla.org/comm-central/source/mozilla/extensions/venkman/locales/all-locales

r+ with those en-US entries mentioned removed.

Comment 11

[Edmund Wong (:ewong)]

Attachment: Correctly remove outdated L10n files on update. (v5) [Checkin: comment 14]

Comment 12

[Jens Hatlak (:InvisibleSmiley)]

What's the situation here? Reading comment 2 suggests that "v1: add needed defines" cannot land since the Core bug didn't land. Still that attachment has not been obsoleted. Thus I don't know what actually needs to land, and whether all dependencies have been met.

Clearing checkin-needed request until the above is resolved.

Comment 13

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 520488 [details] [diff] [review]
v1: add needed defines

Callek wrote via PM: "Just ewongs patch" -> obsoleting first patch

Comment 14

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 530522 [details] [diff] [review]
Correctly remove outdated L10n files on update. (v5) [Checkin: comment 14]

http://hg.mozilla.org/comm-central/rev/87dfb7e86938
http://hg.mozilla.org/releases/comm-2.0/rev/35ac22905535

Comment 15

[Robert Kaiser]

Yes, thanks, we want ewongs patch as mine would depend on a Mozilla fix we didn't get yet, not in trunk and of course not in mozilla-2.0 - thanks for taking care!