Last Comment Bug 1153327 - Run compare-locales with merging for Lightning
: Run compare-locales with merging for Lightning
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: Build Config (show other bugs)
: Trunk
: x86 Mac OS X
-- normal (vote)
: ---
Assigned To: Philipp Kewisch [:Fallen]
:
:
Mentors:
Depends on:
Blocks: 1130854
  Show dependency treegraph
 
Reported: 2015-04-10 10:10 PDT by Philipp Kewisch [:Fallen]
Modified: 2015-04-28 08:40 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fix - v1 (7.62 KB, patch)
2015-04-10 10:29 PDT, Philipp Kewisch [:Fallen]
mh+mozilla: review+
Details | Diff | Splinter Review

Description User image Philipp Kewisch [:Fallen] 2015-04-10 10:10:44 PDT
Right now we require all locales to be complete, it would be better to use l10n-merge/compare-locales to fall back to English strings.

This is also preparation for l10n-repacks to work, hence I needed to make the ABI flag lowercase.
Comment 1 User image Philipp Kewisch [:Fallen] 2015-04-10 10:20:20 PDT
Pushed to comm-central changeset 9aa843cbd62d
Comment 2 User image Philipp Kewisch [:Fallen] 2015-04-10 10:20:56 PDT
argh, sorry. That was a try push.
Comment 3 User image Philipp Kewisch [:Fallen] 2015-04-10 10:29:51 PDT
Created attachment 8590953 [details] [diff] [review]
Fix - v1
Comment 5 User image Philipp Kewisch [:Fallen] 2015-04-12 02:12:29 PDT
Comment on attachment 8590953 [details] [diff] [review]
Fix - v1

Taking back review for now, still errors with unification *sigh*. We might just have to dump this in once there is a green try build and see how it goes for the beta.
Comment 6 User image Philipp Kewisch [:Fallen] 2015-04-13 12:53:06 PDT
Comment on attachment 8590953 [details] [diff] [review]
Fix - v1

This one is good to go, green(ish) try run here, requires bug 1153790. Should be landed together with bug 1130854.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=da24c38fc3b5
Comment 7 User image Joshua Cranmer [:jcranmer] 2015-04-14 22:01:25 PDT
Comment on attachment 8590953 [details] [diff] [review]
Fix - v1

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

This is non-trivial changes to l10n repack code, which is a part of the build system that I'm quite frankly not competent to review.
Comment 8 User image Philipp Kewisch [:Fallen] 2015-04-19 14:19:24 PDT
There is not really a lot of magic to it. All I am changing here is running compare-locales for Lightning from within the build process (because we can't hook it into mozharness) and making sure LOCALE_MERGEDIR is set when running the Lightning targets. This adds --locale-mergedir=$(LOCALE_MERGEDIR) to the jar packager and therefore locale files are packed with merged English strings (http://mxr.mozilla.org/comm-central/source/mozilla/config/config.mk#572)

The remaining changes are just to make the build system and packagers happy as well as some cleanup: Some part of the packager fails because we had ABI uppercase instead of lower case. Also, I don't know why it worked before, the original practice of putting multiple pattern rules on the left side of the rule does something different and I needed to fix it to have the targets called correctly.

I don't really mind who reviews it, but I'd appreciate if we could get this in soon (Monday/Tuesday?) so it can be part of beta 3. Joshua, to take some burden from glandium who has already been putting up with enough of my patches lately, maybe you want to reconsider?

Note You need to log in before you can comment on or make changes to this bug.