Closed Bug 1337825 Opened 3 years ago Closed 3 years ago

Schedule l10n repacks for fennec with specified locale revisions using an intree changesets file

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Callek, Assigned: jlorenzo)

References

Details

Attachments

(2 files, 1 obsolete file)

This bug requires parsing the .json changesets file

And then passing in the explicit revisions to the mozharness commands for locales, via the task graph.
See Also: → 1337875
This comes as a surprise to me. How is this intended to pick up changes from the l10n dashboard, and how does the timing look?
Comment on attachment 8838130 [details]
Bug 1337825 - Jamun: Change android-api-15-{l10n,nightly} to l10n bumped file

Oops, some tweak are still needed https://tools.taskcluster.net/task-group-inspector/#/D3YOpqk6QkiGHAMC3jEmyg/G3g88SHAS4aaLLDaLneYCA?_k=o3gey8 . Even though I correctly pass the revision, I need to point it to the correct repo.
Attachment #8838130 - Flags: review?(aki)
:pike - bug 1339641 and bug 1337645.  All in-tree.
Comment on attachment 8838123 [details]
Bug 1337825 - Schedule l10n repacks for fennec with specified locale revisions using an intree changesets file

https://reviewboard.mozilla.org/r/113124/#review114584

Once we have l10n-changesets.json on mozilla-central and mozilla-aurora, we'll want this patch on both of them.
r+ with nits; let me know if you want me to change the platforms list in l10n-changesets.json.

::: taskcluster/docs/attributes.rst:135
(Diff revision 1)
>  of relevant locales for the platform.
>  
> +all_locales_with_changesets
> +===========================
> +
> +Contains a dict of l10n changesets, mappyed by locales (same as in ``all_locales``).

*mapped

::: taskcluster/taskgraph/transforms/l10n.py:144
(Diff revision 1)
>      locales = []
> +
> +    with open(locales_file, mode='r') as f:
> -    if locales_file.endswith('json'):
> +        if locales_file.endswith('json'):
> -        # Release process uses .json for locale files sometimes.
> -        raise NotImplementedError("Don't know how to parse a .json locales file")
> +            all_locales = json.load(f)
> +            single_locales = {locale: data['revision'] for locale, data in all_locales.items() if 'android' in data['platforms']}

We'll probably have to deal with this hardcode at some point.  Would you prefer I put android-api-15 and android-x86 in the platforms list?  We can do that.

Also, do you want to set `locales` here? We don't have to specify `multi_locales` so that change would simply this block.

::: taskcluster/taskgraph/transforms/l10n.py:157
(Diff revision 1)
>  
> +def _remove_ja_jp_mac_locale(locales):
> +    # ja-JP-mac is a mac-only locale, but there are no mac builds being repacked, so just omit it unconditionally
> +    return {
> +        locale: revision for locale, revision in locales.items() if locale != 'ja-JP-mac'
> +    }

If we do standardize on the json, we would eliminate ja-JP-mac from non-mac platforms by the `platforms` key/value pair.  It's harmless for now.
Attachment #8838123 - Flags: review?(aki) → review+
Comment on attachment 8838130 [details]
Bug 1337825 - Jamun: Change android-api-15-{l10n,nightly} to l10n bumped file

https://reviewboard.mozilla.org/r/113126/#review114630

::: testing/mozharness/configs/multi_locale/jamun_android-armv6.json:5
(Diff revision 2)
> +{
> +    "work_dir": ".",
> +    "log_name": "multilocale",
> +    "objdir": "obj-firefox",
> +    "locales_file": "build/mobile/android/locales/maemo-locales",

here too.

::: testing/mozharness/configs/multi_locale/jamun_android-x86.json:5
(Diff revision 2)
> +{
> +    "work_dir": ".",
> +    "log_name": "multilocale",
> +    "objdir": "obj-firefox",
> +    "locales_file": "build/mobile/android/locales/maemo-locales",

and here.

::: testing/mozharness/configs/multi_locale/jamun_android.json:5
(Diff revision 2)
> +{
> +    "work_dir": ".",
> +    "log_name": "multilocale",
> +    "objdir": "obj-firefox",
> +    "locales_file": "build/mobile/android/locales/maemo-locales",

We'll want to a) point this at `l10n-changesets.json`, and b) specify `"locales_platform": "android-multilocale",` per https://reviewboard.mozilla.org/r/112934/diff/1#index_header 

If you want to do that here, great.  If you want me to update these files when I get the rest of bug 1339706 ready, let me know.

::: testing/mozharness/configs/single_locale/jamun_android-api-15.py:15
(Diff revision 2)
> +    "objdir": "obj-l10n",
> +    "is_automation": True,
> +    "buildbot_json_path": "buildprops.json",
> +    "force_clobber": True,
> +    "clobberer_url": "https://api.pub.build.mozilla.org/clobberer/lastclobber",
> +    "locales_file": "%s/mobile/android/locales/all-locales" % MOZILLA_DIR,

I think we can specify the l10n-changesets file here, and specify the locales_platform as `android` or `android-api-15` depending whether we change the platforms list.  However, since we're explicitly passing in the `--locales`, I believe this is ignored.
Attachment #8838130 - Flags: review?(aki) → review+
Attachment #8838130 - Attachment is obsolete: true
Comment on attachment 8838513 [details]
Bug 1337825 - Jamun: Change android-api-15-{l10n,nightly} to l10n bumped file

Crap, I messed up with mozreview. This patch is attachment 8838130 [details] + review comments addressed. Carrying over r+.

I addressed them all, with the exception of:
> We'll want to a) point this at `l10n-changesets.json`,
done

> and b) specify `"locales_platform": "android-multilocale",` per https://reviewboard.mozilla.org/r/112934/diff/1#index_header 
I left this change for bug 1339706

> and specify the locales_platform as android or android-api-15
Like above, it should be done in bug 1339706
Attachment #8838513 - Flags: review?(aki) → review+
Latest changes tested against:
* Jamun (where locale comes from l10n-changeset.json): https://tools.taskcluster.net/task-group-inspector/#/R8V_mbnuQoasxeuMzGwgaw
* Date (where locales are from the all-locale file, and hence using "default" as revision for l10n repos): https://tools.taskcluster.net/task-group-inspector/#/A1mferJjTk-uWbkDRyJ6RA

I'm gonna land attachment 8838123 [details] in central. This needs to ride aurora, so leaving the bug open.

I won't land attachment 8838513 [details] until l10n-changesets.json is present in central and aurora. Keeping this bug open for this reason too.
Pushed by jlorenzo@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7c9f7712163
Schedule l10n repacks for fennec with specified locale revisions using an intree changesets file r=aki
sorry had to back out for flake 8 error like https://treeherder.mozilla.org/logviewer.html#?job_id=78299256&repo=mozilla-inbound
Flags: needinfo?(jlorenzo)
Pushed by jlorenzo@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/17d5fa614caa
Schedule l10n repacks for fennec with specified locale revisions using an intree changesets file r=aki
Thank you for spotting this error :Tomcat. I just relanded a flake8-error-free revision[1].

[1] Tested locally with `mach lint -l flake8 -f treeherder`
Flags: needinfo?(jlorenzo)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d226bf0c5b39
Backed out changeset c7c9f7712163 for flake8 failure
Jamun now has the new l10n-changesets.json format, if you want to key off 'android-api-15' in platforms.
I plan on having the new l10n-changesets.json in m-c and m-a earlyish next week, though I'm off Monday.
https://hg.mozilla.org/projects/jamun/rev/c26b44d6efa1
Discussed with Aki, we should also land the Jamun changes on central and aurora. I applied attachment 8838513 [details] on top of inbound and run all the linters[1]. No errors related to the patch were detected. I pushed the patch on inbound. 

[1] `./mach lint -f treeherder`
Pushed by jlorenzo@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/405ee7f97cfa
Jamun: Change android-api-15-{l10n,nightly} to l10n bumped file r=aki
Attachment 8838513 [details] landed in aurora at https://hg.mozilla.org/releases/mozilla-aurora/rev/982aaad2e5d766031aa81e7f4366524bdae950f4

All the changes will now ride the trains. This bug can now be closed.
Status: NEW → RESOLVED
Closed: 3 years ago
Keywords: leave-open
Resolution: --- → FIXED
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.