Closed Bug 1424146 Opened 3 years ago Closed 3 years ago

Simplify taskcluster l10n mozharness config.

Categories

(Release Engineering :: Applications: MozharnessCore, enhancement)

enhancement
Not set
normal

Tracking

(firefox60 fixed)

RESOLVED FIXED
Tracking Status
firefox60 --- fixed

People

(Reporter: tomprince, Assigned: tomprince)

References

Details

Attachments

(6 files, 1 obsolete file)

No description provided.
Comment on attachment 8936211 [details]
Bug 1424146: Simplify taskcluster l10n mozharness config;

https://reviewboard.mozilla.org/r/206524/#review213594

Update for merge from maple.
Comment on attachment 8936211 [details]
Bug 1424146: Simplify taskcluster l10n mozharness config;

https://reviewboard.mozilla.org/r/206526/#review219010

::: taskcluster/ci/nightly-l10n/kind.yml:216
(Diff revision 4)
> -               - environment-config=single_locale/production.py
> -               - branch-config=single_locale/{project}.py
> -               - platform-config=single_locale/win64.py
> -               - config=single_locale/tc_win64.py
> -               - config=taskcluster_nightly.py
> -               - revision=$GECKO_HEAD_REV
> +         - revision=$GECKO_HEAD_REV

note: windows is the only platform specifying revision this way. I *think* it will work for other platforms, but is something to watch out for. Alternatively you can change this taskgraph gen to only specify it for windows if you want to be cautious.
Attachment #8936211 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 8937093 [details]
Bug 1424146: Refactor taskcluster l10n mozharness config;

https://reviewboard.mozilla.org/r/207792/#review219012

I'm less keen to *add* a new config file in the mix here, given how confusing l10n configs are already. But since this helps unblock TB l10n, then I'm all for it.

Of note as well is that update_gecko and the buildbot_json stuff was only in the windows configs here, but I don't mind this.

--I also wonder if there are conflicts on m-c that make this unable to land cleanly (like stuff for deved/release promotion) if so and you need a second eye feel free to ping or n-i me.
Attachment #8937093 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 8937093 [details]
Bug 1424146: Refactor taskcluster l10n mozharness config;

https://reviewboard.mozilla.org/r/207792/#review219012

> I'm less keen to add a new config file in the mix here, given how confusing l10n configs are already.

Yeah. My hope is that eventually the non-tc stuff can just go away, and we can remove a bunch of config files.

> Of note as well is that update_gecko and the buildbot_json stuff was only in the windows configs here, but I don't mind this.

Yeah. I suspect that the windows stuff may have gotten more care. I do think the settings are correct for TC builds on all platforms.

> I also wonder if there are conflicts on m-c

There are no code conflicts -- I've been been rebasing this regularlly for testing, but I'll to a try run before landing.
Comment on attachment 8936211 [details]
Bug 1424146: Simplify taskcluster l10n mozharness config;

https://reviewboard.mozilla.org/r/206526/#review219010

> note: windows is the only platform specifying revision this way. I *think* it will work for other platforms, but is something to watch out for. Alternatively you can change this taskgraph gen to only specify it for windows if you want to be cautious.

It has been awhile since I wrote this code. But I *think* this change is necessary, with the change in the next commit to disable `update_gecko_source_to_enUS`. (based on the code [here](https://searchfox.org/mozilla-central/rev/48cbb200aa027a0a379b6004b6196a167344b865/testing/mozharness/scripts/desktop_l10n.py#489-499), mozharness needs the revision passed, if it can't get from buildbot and isn't updating to match `en-US`).
Comment on attachment 8943873 [details]
Bug 1424146: Remove mock configuration from linux L10N jobs; r?Callek

Justin Wood (:Callek) has approved the revision.

https://phabricator.services.mozilla.com/D417#9938
Attachment #8943873 - Flags: review+
Comment on attachment 8943874 [details]
Bug 1424146: Remove unused configuration from L10N jobs; r?Callek

Justin Wood (:Callek) has approved the revision.

https://phabricator.services.mozilla.com/D418#9952
Attachment #8943874 - Flags: review+
Comment on attachment 8943876 [details]
Bug 1424146: Remove unused `exes.virtualenv` from mozharness configs; r?jlund

Jordan Lund (:jlund) has approved the revision.

https://phabricator.services.mozilla.com/D420#10001
Attachment #8943876 - Flags: review+
Comment on attachment 8943875 [details]
Bug 1424146: Remove unused `tooltool_script` from L10N and repackage jobs; r?Callek

[n.b. not sure how I'm not flagged on this one...]
Attachment #8943875 - Flags: review?(bugspam.Callek)
Comment on attachment 8936211 [details]
Bug 1424146: Simplify taskcluster l10n mozharness config;

https://reviewboard.mozilla.org/r/206526/#review219010

> It has been awhile since I wrote this code. But I *think* this change is necessary, with the change in the next commit to disable `update_gecko_source_to_enUS`. (based on the code [here](https://searchfox.org/mozilla-central/rev/48cbb200aa027a0a379b6004b6196a167344b865/testing/mozharness/scripts/desktop_l10n.py#489-499), mozharness needs the revision passed, if it can't get from buildbot and isn't updating to match `en-US`).

Ah-ha I found the trick I was thinking of here. so, it *is* passed on linux and mac now, its just not specified in the kind.yml (I'm not sure if specifying it twice will be confusing or breaking, but I'm open to whatever you do here, as this whole code area needs more cleanup still)

https://dxr.mozilla.org/mozilla-central/source/taskcluster/scripts/builder/build-l10n.sh#99
Comment on attachment 8936211 [details]
Bug 1424146: Simplify taskcluster l10n mozharness config;

https://reviewboard.mozilla.org/r/206526/#review219010

> Ah-ha I found the trick I was thinking of here. so, it *is* passed on linux and mac now, its just not specified in the kind.yml (I'm not sure if specifying it twice will be confusing or breaking, but I'm open to whatever you do here, as this whole code area needs more cleanup still)
> 
> https://dxr.mozilla.org/mozilla-central/source/taskcluster/scripts/builder/build-l10n.sh#99

Ah. That isn't a place I'd expect to find configuration. I'll remove that, so we avoid passing it twice.
Attachment #8937093 - Attachment is obsolete: true
> [n.b. not sure how I'm not flagged on this one...]

Phabricator only mirrors `r+`, not any other status. (http://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#review-flags)
Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7de6df7d8a5
Simplify taskcluster l10n mozharness config; r=Callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/765696fa83d9
Refactor taskcluster l10n mozharness config; r=Callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/54a176106302
Remove mock configuration from linux L10N jobs; r=Callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa32c080a937
Remove unused configuration from L10N jobs; r=Callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/1eb34cd4d331
Remove unused `tooltool_script` from L10N and repackage jobs; r=Callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/14286b23800c
Remove unused `exes.virtualenv` from mozharness configs; r=jlund
Comment on attachment 8944438 [details]
Bug 1424146: Refactor taskcluster l10n mozharness config;

https://reviewboard.mozilla.org/r/214636/#review221774
Attachment #8944438 - Flags: review?(bugspam.Callek) → review+
Attachment #8943875 - Flags: review?(bugspam.Callek) → review+
You need to log in before you can comment on or make changes to this bug.