1195865 - Upload android single-locale repacks to Taskcluster

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Michael Shal [:mshal]]

Similar to bug 1174241, we need to have mobile_l10n.py upload artifacts to Taskcluster.

Comment 1

[Michael Shal [:mshal]]

Attachment: bbconfigs-locale-environment

I'd like to use the single_locale/{staging|production}.py files for android l10n builds. Currently we don't which means there is no appropriate taskcluster_index variable in the config, and the upload environment is hardcoded to production so I have to patch my tree to use it in staging.

Comment 2

[Michael Shal [:mshal]]

Attachment: bbcustom-locale-environment

The staging/production config currently has to be first, at least until mozharness is patched. The android single-locale configs override the upload_env anyway, so this is somewhat of a no-op for now. Once we're passing in staging.py/production.py, I can remove the upload_env from mozharness' single_locale/*android*.py

Comment 3

[[e-mail] (:bhearsum)]

Comment on attachment 8649970 [details] [diff] [review]
bbconfigs-locale-environment

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

::: mozilla/production_config.py
@@ +63,5 @@
>      'tooltool_url_list': ['https://api.pub.build.mozilla.org/tooltool/'],
>      'blob_upload': True,
>      'mozharness_configs': {
>          'balrog': 'balrog/production.py',
> +        'locale_environment': 'single_locale/production.py',

Naming nit: maybe single_locale_environment instead? I'm fine either way though.

Comment 4

[Michael Shal [:mshal]]

Comment on attachment 8649970 [details] [diff] [review]
bbconfigs-locale-environment

https://hg.mozilla.org/build/buildbot-configs/rev/bdb8c9357d03

Landed with the name change.

Comment 5

[Michael Shal [:mshal]]

Comment on attachment 8649975 [details] [diff] [review]
bbcustom-locale-environment

https://hg.mozilla.org/build/buildbotcustom/rev/3c6738555c9e

Landed with the name change.

Comment 6

[Michael Shal [:mshal]]

Attachment: 0001-Bug-1195865-Remove-default_actions-from-android-l10n.patch

These default_actions are redundant with the list in mobile_l10n.py, and aren't included in all the android l10n configs, just the release ones for some reason.

Comment 7

[Michael Shal [:mshal]]

Attachment: 0002-Bug-1195865-Use-upload-environment-from-staging.py-p.patch

Use the upload environment from staging.py/production.py instead of hard-coding the production environment in all the android l10n configs. The slightly tricky parts are the fact that the branch for android is different than desktop, since it also includes the platform name. Also MOZ_PKG_VERSION only seems to be present in the release builds.

Comment 8

[Michael Shal [:mshal]]

Attachment: 0003-Bug-1195865-Upload-android-l10n-repacks-to-Taskclust.patch

This was built on top of the pushlog patches, so I may need to update it slightly based on the outcome of bug 1194709.

Comment 9

[Rail Aliiev [:rail]]

Comment on attachment 8650643 [details] [diff] [review]
0002-Bug-1195865-Use-upload-environment-from-staging.py-p.patch

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

lgtm

Comment 10

[Jordan Lund (:jlund)]

Comment on attachment 8650645 [details] [diff] [review]
0003-Bug-1195865-Upload-android-l10n-repacks-to-Taskclust.patch

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

neat. this is great how you are going across all the scripts and doing this.

::: testing/mozharness/scripts/mobile_l10n.py
@@ +477,5 @@
> +        # We need to activate the virtualenv so that we can import taskcluster
> +        # (and its dependent modules, like requests and hawk).  Normally we
> +        # could create the virtualenv as an action, but due to some odd
> +        # dependencies with query_build_env() being called from build(), which
> +        # is necessary before the virtualenv can be created.

do you mean s/which is/this is/ ?

bear with me, how does query_build_env() play a part here?

@@ +489,5 @@
> +        make = self.query_exe("make")
> +        upload_env = self.query_upload_env()
> +        cwd = dirs['abs_locales_dir']
> +        branch = self.config['branch']
> +        platform = self.config['platform']

platform doesn't look to be used anymore

@@ +510,5 @@
> +            abs_files = [os.path.abspath(os.path.join(cwd, f)) for f in files]
> +
> +            routes = []
> +            for template in templates:
> +                fmt = {

tiny nit: iiuc, fmt's init() and update() be defined once outside of the templates iteration

@@ +519,5 @@
> +                    'build_name': self.query_build_name(),
> +                    'build_type': self.query_build_type(),
> +                    'locale': locale,
> +                }
> +                fmt.update(self.buildid_to_dict(self.query_buildid()))

is the buildid's parts used at all. Maybe I'm looking at the wrong file: http://mxr.mozilla.org/mozilla-central/source/testing/taskcluster/routes.json#12

I sort of recall this routes.json and you were going to be adding buildid date items to the index. ignore me if you are just prepping for that here

Comment 11

[Release Engineering SlaveAPI Service]

In production: https://hg.mozilla.org/build/buildbot-configs/rev/bdb8c9357d03

Comment 12

[Release Engineering SlaveAPI Service]

In production: https://hg.mozilla.org/build/buildbotcustom/rev/3c6738555c9e

Comment 13

[Michael Shal [:mshal]]

(In reply to Jordan Lund (:jlund) from comment #10)
> ::: testing/mozharness/scripts/mobile_l10n.py
> @@ +477,5 @@
> > +        # We need to activate the virtualenv so that we can import taskcluster
> > +        # (and its dependent modules, like requests and hawk).  Normally we
> > +        # could create the virtualenv as an action, but due to some odd
> > +        # dependencies with query_build_env() being called from build(), which
> > +        # is necessary before the virtualenv can be created.
> 
> do you mean s/which is/this is/ ?
> 
> bear with me, how does query_build_env() play a part here?

Hmm, it seems this issue is specific to fx_desktop_build.py. I tried using 'create-virtualenv' as an action before taskcluster-upload, and that seems to work for the l10n scripts. I'll re-request review for this change.

> 
> @@ +489,5 @@
> > +        make = self.query_exe("make")
> > +        upload_env = self.query_upload_env()
> > +        cwd = dirs['abs_locales_dir']
> > +        branch = self.config['branch']
> > +        platform = self.config['platform']
> 
> platform doesn't look to be used anymore

Fixed!

> @@ +510,5 @@
> > +            abs_files = [os.path.abspath(os.path.join(cwd, f)) for f in files]
> > +
> > +            routes = []
> > +            for template in templates:
> > +                fmt = {
> 
> tiny nit: iiuc, fmt's init() and update() be defined once outside of the
> templates iteration

Fixed!

> @@ +519,5 @@
> > +                    'build_name': self.query_build_name(),
> > +                    'build_type': self.query_build_type(),
> > +                    'locale': locale,
> > +                }
> > +                fmt.update(self.buildid_to_dict(self.query_buildid()))
> 
> is the buildid's parts used at all. Maybe I'm looking at the wrong file:
> http://mxr.mozilla.org/mozilla-central/source/testing/taskcluster/routes.
> json#12

You're right, it isn't used for the l10n routes, so I removed it for now. We may want to add nightly-style routes for l10n in the future, but we can put this back in then if so.

Comment 14

[Michael Shal [:mshal]]

Attachment: 0003-Bug-1195865-Upload-android-l10n-repacks-to-Taskclust.patch

Requesting re-review for the virtualenv changes.

Comment 15

[Jordan Lund (:jlund)]

Comment on attachment 8652905 [details] [diff] [review]
0003-Bug-1195865-Upload-android-l10n-repacks-to-Taskclust.patch

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

looks great :)

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d2f924ff750
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc9c846f8598
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c9a4b6596e6

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Backed out for B2G build bustage.
https://treeherder.mozilla.org/logviewer.html#?job_id=13339604&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/81f00129aedb

Comment 18

[Michael Shal [:mshal]]

Attachment: 0001-query_repo-query_l10n_repo.patch

So this ended up breaking B2G builds, since the query_repo() in locales.py ended up overriding the query_repo() in buildb2gbase.py. This is a fixup to change the name of the locales version to query_l10n_repo(). Also I intended to use this in desktop_l10n.py, so I removed that version. Things look fine on the staging l10n builds I've done, and I'm currently running it on try.

Comment 19

[Michael Shal [:mshal]]

I'll fold it into the previous commit if it looks good - just figured it'd be easier to review this way.

Comment 20

[Michael Shal [:mshal]]

Here's try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e711052f76c7

just looks messy because of the vcs issues :)

Comment 21

[Jordan Lund (:jlund)]

Comment on attachment 8653147 [details] [diff] [review]
0001-query_repo-query_l10n_repo.patch

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

thanks for diffing it like this.

sorry you are having mixin fun :)

Comment 22

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/30be2da3c090
https://hg.mozilla.org/integration/mozilla-inbound/rev/c57395522d53
https://hg.mozilla.org/integration/mozilla-inbound/rev/53ac5605d91a

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/30be2da3c090
https://hg.mozilla.org/mozilla-central/rev/c57395522d53
https://hg.mozilla.org/mozilla-central/rev/53ac5605d91a