1136370 - Enable pseudolocales by default for dev builds

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::L10n, defect)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

Pseudolocales should be enabled by default for dev builds.

Comment 1

[Staś Małolepszy :stas]

Let's define what "dev" builds are.  Do you mean custom builds done by developers with 'make install-gaia' or 'make reset-gaia' or 'make production'?  In this case, qps-* are already defined in shared/resources/languages.json and we'd just need to add 'devtools.qps.enabled: true' somewhere to the build config for this particular type of builds.

I'm not sure how build/config/common-settings.json is used.  Is it also used by production builds?

Another option is to enable qps in -eng builds from pvt.  AFAIK, they use LOCALES_FILE=locales/languages_*.json so we could start by adding qps-* to those files.  I'm also not sure if we need both languages_dev.json and languages_all.json these days, so maybe this is an opportunity to remove one of them.

We'd need to set 'devtools.qps.enabled: true' for the *-eng builds too.

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] KevinGrandon:bug_1136370_pseudo_locales_in_eng_builds > mozilla-b2g:master

Comment 3

[Kevin Grandon :kgrandon]

Comment on attachment 8568793 [details] [review]
[gaia] KevinGrandon:bug_1136370_pseudo_locales_in_eng_builds > mozilla-b2g:master

I was just thinking something simple like this. It will allow the runtime pseudolocales to show up in the FTU language list, and the option will be enabled by default in the settings app. This should happen for all "non-production" builds. Thoughts?

Comment 4

[Staś Małolepszy :stas]

Comment on attachment 8568793 [details] [review]
[gaia] KevinGrandon:bug_1136370_pseudo_locales_in_eng_builds > mozilla-b2g:master

I think this is okay, but I want to make sure we're on the same page here.  This setting is a little bit ambiguous and I take the blame.  All it does is control whether pseudolocales should be listed in Settings > Language.  Then, if they were built on buildtime, it works just like regular locales do.  If they're not built, l10n.js will use runtime generation logic, slowing things down a little bit.

With this patch:

 - if LOCALES_FILE (shared/resources/languages.json by default) includes qps-*, the enabled pseudolocales will be built on buildtime,

 - otherwise, they will be generated on runtime.  This will be the case for builds at [1] since they use (an outdated, it would seem) locales/languages_all.json as LOCALES_FILE.

I think it would make sense to add both qps locales to locales/languages_all.json in your pull request and work with releng to make sure the newest version is used for builds at [1].

What do you think?

[1] https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-central-flame-kk-eng/latest/

Comment 5

[Kevin Grandon :kgrandon]

Stas - the only thing I'm concerned about is that it feels wrong to include these in a languages json file for a PRODUCTION profile, and I didn't see a good way of including these based on the PRODUCTION flag. I am worried that we might build and ship the pseudolocales for a PRODUCTION device when we don't want to.

I think that maybe we should just go with the runtime solution for now? It should be possible to go with build-time pseudolocales, but I think we need to generate the json files based on the PRODUCTION flag for that?

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Stas, unless we revisit my idea of using a flag to generate build time pseudolocales instead of relying on LOCALES_FILE. This way you can bind the flag to PRODUCTION and build pseudo when PRODUCTION=0 no matter of LOCALES_FILE content.

Comment 7

[Kevin Grandon :kgrandon]

Comment on attachment 8568793 [details] [review]
[gaia] KevinGrandon:bug_1136370_pseudo_locales_in_eng_builds > mozilla-b2g:master

Ok, this seems like the "simple" fix for now, so I'm going to ask a build peer to take a look. Comment 6 seems like a decent solution, but it also seems like a bit of work, and something that we can do after landing this patch first.

Ricky - could you give this a quick review if you have the time?

Comment 8

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8568793 [details] [review]
[gaia] KevinGrandon:bug_1136370_pseudo_locales_in_eng_builds > mozilla-b2g:master

LGTM.

Comment 9

[Kevin Grandon :kgrandon]

Ok, let's go with enabling just runtime pseudolocales in this bug, and we can file another bug to implement buildtime pseudolocales if that would be better.

In master: https://github.com/mozilla-b2g/gaia/commit/3a57533fa8a6cbc4c99409103efeff211a838d04

Comment 10

[Staś Małolepszy :stas]

(In reply to Kevin Grandon :kgrandon from comment #5)
> Stas - the only thing I'm concerned about is that it feels wrong to include
> these in a languages json file for a PRODUCTION profile, and I didn't see a
> good way of including these based on the PRODUCTION flag. I am worried that
> we might build and ship the pseudolocales for a PRODUCTION device when we
> don't want to.

I'm not sure thinking in terms of PRODUCTION builds is the right approach here. locales/languages_all.json is not, by all means, a file to be used as the value of LOCALES_FILE in production builds.  It has 86 languages in it, many of them extremely incomplete.  All partners use their own LOCALES_FILE tailored to the regions they ship in.

And if someone does use it for production builds, it means that they have plenty of space available for those 86 locales, and adding two more shouldn't be a problem.

So again, I think this patch should have added qps-* locales to languages_all.json.  I should have f-'ed it to make this more explicit.  Can you file a follow up please?

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

Stas: I still think that you an I disagree on that. Seeing this bug makes me a bit more certain about my comment 6. qps should be triggered by a flag, orthogonally to languages listed in LOCALES_FILE and bound to PRODUCTION.

Comment 12

[Staś Małolepszy :stas]

I think I like to separate the 'how' and the 'what' when it comes to the build config.  The 'what' is all the things we include in the build:

 - apps: https://github.com/mozilla-b2g/gaia/tree/1ff734dba9812f34782b8dabb1ce3b2bdb7ca0e3/build/config/phone
 - settings: https://github.com/mozilla-b2g/gaia/blob/1ff734dba9812f34782b8dabb1ce3b2bdb7ca0e3/build/config/common-settings.json + custom-settings.json
 - prefs: build/config/custom-prefs.js
 - languages: shared/resources/languages.json

The 'how' is how we do it:

 - uglify JS?  GAIA_OPTIMIZE=1
 - concatenate l10n resources?  GAIA_CONCAT_LOCALES=1
 - pretranslate HTML file?  GAIA_PRETRANSLATE=1

There's clearly some overlap and some legacy artifacts, too.  E.g. a long time ago I added REMOTE_DEBUGGER but now it's much more convenient to use build/config/custom-settings.json to set the 'debugger.remote-mode' setting.  So now I wish I hadn't added it, because it just sits there polluting the Makefile but maybe it's used by someone somewhere and we don't know if we can safely remove it without breaking anything. 

Makefile flags seem to be well suited for the 'how' and used for commonly-used umbrella 'whats' like PRODUCTION, DOGFOOD or DEMO, which btw are set indirectly via MAKECMDGOALS.

Following this logic, I'm not a fan of GAIA_KEYBOARD_LAYOUTS because it looks like it should be a file in build/config.  Some other flags make sense also because they're frequently used: BUILD_APP_NAME or NOFTU would be good examples.

I think LOCALES_FILE is perfectly fine for specifying that we want pseudolocales in the build. I'm opposed to adding yet another Makefile flag because they're genrally not easily discoverable and increase the complexity of the possibility matrix (what does "GAIA_PSEUDOLOCALES=1 GAIA_CONCAT_LOCALES=0" do?).

What I think we could do is have two versions of shared/resources/languages.json and choose the one to be used as LOCALES_FILE based on the value of PRODUCTION, for instance:

https://github.com/mozilla-b2g/gaia/blob/1ff734dba9812f34782b8dabb1ce3b2bdb7ca0e3/Makefile#L436

And I still think we should add qps-* to locales/languages_all.json.

Comment 13

[Zibi Braniecki [:zbraniecki][:gandalf]]

> I think LOCALES_FILE is perfectly fine for specifying that we want pseudolocales in the build.

I disagree with you.

LOCALES_FILE is for listing locales you want to have in your build. Pseudo is not a locale, it's a functionality of your build. You may want to have the *feature* in any kind of build irrelevant of which locales you want to have there.

> what does "GAIA_PSEUDOLOCALES=1 GAIA_CONCAT_LOCALES=0" do?

That's actually easy - you can say that PSEUDOLOCALES require CONCAT for build time, otherwise you'll only have pseudo runtime.

There are more problems with your approach:

 - it requires vendors to define if they want to add pseudolocales by adding them or removing from LOCALES_FILE. I don't think that this is what they expect. Since pseudo is perceived by everyone I talk to, except of you, as a feature, not a language, they would probably expect to turn on/off this feature like any other (or let Gaia devs defaults stay) irrelevant of their choices of bundles locales.
 - it is confusing. We suddenly have "qps-ploc" in our buildsystem as we look through locales and causes all funny errors like:

[multilocale] Resource file not found: /Users/zbraniecki/projects/gaia-l10n/master/qps-ploc/apps/wappush/wappush.properties
[multilocale] Resource file not found: /Users/zbraniecki/projects/gaia-l10n/master/qps-plocm/apps/wappush/wappush.properties

Sure, we can start muting those, specialcasing qps-ploc/qps-plocm, but I believe that it's missing a point.

Once again, restating my position:

 qps-ploc/qps-plocm should not be on the list of locales and not be in the LOCALES_FILE. It should have a build time flag to turn it on/off, and turning it on should trigger the path in the build system that adds the bundled resources and turn on the pseudo flag at runtime.