Closed Bug 1195865 Opened 4 years ago Closed 4 years ago

Upload android single-locale repacks to Taskcluster

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

defect
Not set

Tracking

(firefox43 fixed)

RESOLVED FIXED
Tracking Status
firefox43 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

Details

Attachments

(6 files, 1 obsolete file)

Similar to bug 1174241, we need to have mobile_l10n.py upload artifacts to Taskcluster.
Assignee: nobody → mshal
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.
Attachment #8649970 - Flags: review?(bhearsum)
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
Attachment #8649975 - Flags: review?(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.
Attachment #8649970 - Flags: review?(bhearsum) → review+
Attachment #8649975 - Flags: review?(bhearsum) → review+
Comment on attachment 8649970 [details] [diff] [review]
bbconfigs-locale-environment

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

Landed with the name change.
Attachment #8649970 - Flags: checked-in+
Comment on attachment 8649975 [details] [diff] [review]
bbcustom-locale-environment

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

Landed with the name change.
Attachment #8649975 - Flags: checked-in+
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.
Attachment #8650633 - Flags: review?(rail)
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.
Attachment #8650643 - Flags: review?(rail)
This was built on top of the pushlog patches, so I may need to update it slightly based on the outcome of bug 1194709.
Attachment #8650645 - Flags: review?(jlund)
Attachment #8650633 - Flags: review?(rail) → review+
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
Attachment #8650643 - Flags: review?(rail) → review+
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
Attachment #8650645 - Flags: review?(jlund) → review+
(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.
Requesting re-review for the virtualenv changes.
Attachment #8650645 - Attachment is obsolete: true
Attachment #8652905 - Flags: review?(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 :)
Attachment #8652905 - Flags: review?(jlund) → review+
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.
Attachment #8653147 - Flags: review?(jlund)
I'll fold it into the previous commit if it looks good - just figured it'd be easier to review this way.
Here's try:

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

just looks messy because of the vcs issues :)
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 :)
Attachment #8653147 - Flags: review?(jlund) → review+
You need to log in before you can comment on or make changes to this bug.