1306598 - update l10n kind to not hardcode 'try' in options

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Justin Wood (:Callek)]

This bug is specifically about adding a transform to make the task not hardcode 'try'

+++ This bug was initially created as a clone of Bug #1302200 +++

https://reviewboard.mozilla.org/r/77436/diff/2 breaks the l10n kind out of the legacy kind, but doesn't do the greatest job.

There are some comments there illustrating things to change.  The changes probably span the task-graph generation, the shell scripts that invoke mozharness, and mozharness itself.  Ideally those can be normalized somewhat, so we don't have a separate script to invoke mozharness for l10n.  That may mean adding more options to build-linux.sh.

Comment 1

[Justin Wood (:Callek)]

Attachment: Bug 1306598 - update l10n kind to not hardcode 'try' in options.

Review commit: https://reviewboard.mozilla.org/r/82370/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82370/

Comment 2

[Justin Wood (:Callek)]

Comment on attachment 8796526 [details]
Bug 1306598 - update l10n kind to not hardcode 'try' in options.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82370/diff/1-2/

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8796526 [details]
Bug 1306598 - update l10n kind to not hardcode 'try' in options.

https://reviewboard.mozilla.org/r/82370/#review81028

This is more of a design r-; the code itself works, but I think it's at odds with how transforms are used elsewhere.

Specifically, rather than being "general purpose", transforms are arranged from most- to least-specific.

So I'd like to see

   - taskgraph.transforms.l10n:transforms
   - taskgraph.transforms.build_attrs:transforms
   - taskgraph.transforms.job:transforms
   - taskgraph.transforms.task:transforms

Where the l10n transforms are designed specifically for l10n jobs.  In making that design, think forward to how you can include the minimal information in the yaml files to describe each l10n job.  At a guess, that might be something like

    android-api-15-l10n/opt:
        single-locale: true
        platform: android-api-15
    android-api-15-l10n-full/opt
        locales-per-chunk: 5
        platform: android-api-15

with the rest of the task generated by the transform (including the project).  If there are other "unique" attributes of some l10n jobs, then those could also be in the YAML.  And I'm only guessing here.  The idea is that the yaml is the most concise possible description of the tasks that allows the kind of variation you expect to see in the future.

The thing I'd like to caution you away from is parameter substitution ({project}) in the YAML files.  It suggests the idea of templates, and transforms are meant as an alternative to templates, not as an addition.

Comment 4

[Justin Wood (:Callek)]

Comment on attachment 8796526 [details]
Bug 1306598 - update l10n kind to not hardcode 'try' in options.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82370/diff/2-3/

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8796526 [details]
Bug 1306598 - update l10n kind to not hardcode 'try' in options.

https://reviewboard.mozilla.org/r/82370/#review81496

Minor notes, but this is good to go.

::: taskcluster/taskgraph/transforms/l10n.py:20
(Diff revision 3)
> +        if not job['run'].get('using') == 'mozharness':
> +            # Nothing to do, not mozharness
> +            yield job
> +            continue
> +        if not job['run'].get('config'):
> +            # No config to manipulate
> +            yield job
> +            continue

I think you could safely omit these conditions now

::: taskcluster/taskgraph/transforms/l10n.py:37
(Diff revision 3)
> +        yield job
> +
> +
> [e-mail]
> +def mh_options_replace_project(config, jobs):
> +    """ Replaces {project} in mh config entries with the current project """

..mh options entries..

Comment 6

[Justin Wood (:Callek)]

Comment on attachment 8796526 [details]
Bug 1306598 - update l10n kind to not hardcode 'try' in options.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82370/diff/3-4/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc1926812404
update l10n kind to not hardcode 'try' in options. r=dustin

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/dc1926812404