Fenix: FNX3-16780 ⁃ Allow different locales for Nightly and Release

[pocmo]

Why?

Fenix gets translated on Pontoon and translated strings get imported into this repository "automatically" (kind of - from this team's point of view; @Pike opens PRs for that).

Most of them time it's okay to include all locales into the builds (local builds, Nightly builds), e.g. to allow developers/QA to test the app in other languages and for translators to verify their translations in the app. For other builds (release) we want to explicitly ship a list of "approved" locales.

How?

For release builds we want to run filter-release-translations.py (before invoking Gradle) which will remove all strings of locales that are not in the release list. This will allow engineers and the L10N team to maintain a list of "release locales" in this repository.

Acceptance Criteria

• [ ] Add Release locales list, like in Focus
• [ ] Port filter-release-translations.py from Focus to Fenix and add them to release build run (so only the Release locales get shipped)
• [ ] Verify that locales are filtered out correctly

Next step is actually adding new Nightly locales and making sure they run in #5496

[pocmo]

Dupe of #1723.

[Pike]

Wondering, should this also cover Beta? @Delphine ? I'd suggest so, but not my call.

Also, I found https://developer.android.com/studio/build/shrink-code.html#unused-alt-resources, which might do what we need w/out removing code for release.

I also found posts on stackoverflow indicating that the exposed locale list of the APK might be affected by what we have in dependent libraries like a-c, so we might not get what we expect by just reducing the fenix locales in-tree.

[pocmo]

@Pike I think that is a great idea. :+1: Maintaining the locale list as resConfigs is way less painful - and I assume we can do that for release (and maybe beta) builds separately, so that local builds and Nightlies still have all strings.

[Delphine]

Yes, I agree that we should be covering Beta as well. Thank you!

[severinrudie]

@Pike @Delphine my understanding was that localizers working on partially translated languages should be able to use beta builds to see the translations in app. Is that incorrect? Please verify that we want:

• Release: prune to approved list
• Beta: prune to approved list
• Debug: keep all translations

[liuche]

I'd like (at least before release) for Beta and Release should be as similar as possible, for testing purposes. Localizers will be able see partially localized (but not approved and released) locales on Nightly (and debug, if they wish to build the app).

Once Beta gets released to a larger population (e.g. replacing Fennec Beta) we could change that to go either way.

[severinrudie]

I spent some time working on this with @mcomella today. It looks like resconfig can only be set on the DefaultProductFlavor, afaict there's no exposed way to set it for a particular flavor, and even if we were able to hack one in I'm worried it would leak between configurations. We're looking into using the old Focus script now.

[severinrudie]

It's not very Gradley, but it might be possible to read some environment variable and only set resConfig when building for a release/beta. This could cause strange failures if we ever built multiple formats as part of the same task, but it looks like filter-release-translations.py might as well. I'll reach out to RE and try to find out if this is possible

android {
    defaultConfig {
        if (System.getenv('IS_FOR_PRODUCTION') == "true") {
            resConfigs ...
        }
    }
}

[severinrudie]

@colintheshots mentioned that we could almost handle this via source sets by moving incomplete locales into debug, then adding debug to the nightly resource set. Unfortunately this would not prune any translations from dependencies (namely AC).

[severinrudie]

@Delphine I noticed that our list doesn't include es, only more specific codes (e.g., es-AR). This will cause problems for users who 1) are in a region we don't specify and 2) on older (pre-7) Android versions. They will wind up falling back to default (en) translations.

The easy fix here is to add es to our RELEASE_LOCALES list. If you don't have any objections, I'm going to do that.

[kbrosnan]

Should we do this for other language families? fr for example.

[severinrudie]

Should we do this for other language families? fr for example.

AFAICT, es is the only one we aren't already doing this. It looks like a simple oversight to me, but I wanted to check in before modifying the list.

[Delphine]

Can you point me to your release locale list? I'd like to double-check what's already there, because we don't really have a release process in place yet for any of the other locales. So we'll need to make sure they have been tested by localizers first.

[severinrudie]

@Delphine the list I'm using is here. I took it from the equivalent file in Focus.

[Delphine]

@Baron-Severin - I see, I wasn't aware that we had this list of release locales - and that they came from Focus.
Our current locales translating Fenix can be found here: https://github.com/mozilla-l10n/android-l10n/blob/master/mozilla-mobile/fenix/l10n.toml
I update this list every time I add a locale, so it's probably better to take them from there each time. And as Liuche suggested, we'll prune that list once we get to release.
Does that make sense to you as well?

[severinrudie]

@Delphine sounds good to me, I'll update the PR to point to that. Thanks!

[pocmo]

@Baron-Severin Sorry I had to back out your patch this morning. It was breaking our Nightly builds and therefore a migration build that we needed to get out today.

Here is the log of a failed nightly build: https://firefox-ci-tc.services.mozilla.com/tasks/SYyAtENVRui9A4R3_HBUQw/runs/0/logs/live/https%3A%2F%2Ffirefox-ci-tc.services.mozilla.com%2Fapi%2Fqueue%2Fv1%2Ftask%2FSYyAtENVRui9A4R3_HBUQw%2Fruns%2F0%2Fartifacts%2Fpublic%2Flogs%2Flive.log

[..]
[task 2020-01-17T08:19:53.461Z]     for task in tasks:
[task 2020-01-17T08:19:53.461Z]   File "taskcluster/fenix_taskgraph/transforms/build.py", line 135, in filter_incomplete_translation
[task 2020-01-17T08:19:53.461Z]     task["run"]["pre-gradlew"].append(["python", "automation/taskcluster/l10n/filter-release-translations.py"])
[task 2020-01-17T08:19:53.461Z] KeyError: u'pre-gradlew'

I also wonder if it is a mistake that this runs for Nightly? I assumed we want to have all strings there for translators to check?

[Pike]

I think it's taskcluster-y that the decision task creates the release tasks with filter-incomplete-translations, and then drops them via --target-tasks-method=nightly in graph optimization.

[severinrudie]

I'm not very familiar with TC, so I could be way off the mark, but I wonder if this could be why it ran.

We didn't set pruning to run on any *-nightly tasks, but it was set on fennec-production and fennec-beta, which both have attributes: nightly: true.

As to why it failed, I have no idea. We did a test run here against this PR, and it worked as expected. I'll have to investigate.

[severinrudie]

Context from an email chain:

What's happening is basically: task["run"]["pre-gradlew"] is set here but only for builds that don't require secrets (like PR builds). Nightlies do need secrets so task["run"]["pre-gradlew"] ends up not being set. That's how you get this python error. Making the changes below will make the code work in any types of build.

The fix should be the following:
-            task["run"]["pre-gradlew"].append(["python", "automation/taskcluster/l10n/filter-release-translations.py"])
+            pre_gradlew = task["run"].setdefault("pre-gradlew", [])
+            pre_gradlew.append(["python", "automation/taskcluster/l10n/filter-release-translations.py"])

[severinrudie]

QA note: we will want to verify that no locales are stripped from nightly, and unlisted locales _are_ stripped from release. I don't think this will be possible via manual testing (e.g., we will strip en-GB, but it will fall back to en and look very similar). If it's alright with you, I will verify after the next release is cut, and comment here with the results.

Note: this didn't make it in before v3.2.0 was cut. It'll have to be verified in the next cycle.

[sv-ohorvath]

@Baron-Severin thanks for checking! We'll leave it to you.

[softvision-miralobontiu]

QA note: we will want to verify that no locales are stripped from nightly, and unlisted locales _are_ stripped from release. I don't think this will be possible via manual testing (e.g., we will strip en-GB, but it will fall back to en and look very similar). If it's alright with you, I will verify after the next release is cut, and comment here with the results.

Note: this didn't make it in before v3.2.0 was cut. It'll have to be verified in the next cycle.

Based on this comment, I will remove the eng:qa:needed.

[severinrudie]

Verified as working on release 4.0.

Process:

• Built a local debug build
• Analyzed the APK, verified that string content_description_menu was present in the incomplete iw translation.
• Verified that content_description_menu was missing in the iw locale from a beta APK taken from this TC task.