Closed Bug 1286092 Opened 3 years ago Closed Last year

L10n jobs should run per-push based on the corresponding builds

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(firefox62 fixed)

RESOLVED FIXED
Tracking Status
firefox62 --- fixed

People

(Reporter: glandium, Assigned: Callek)

References

Details

Attachments

(16 files)

46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
Pike
: review+
Details | Review
46 bytes, text/x-phabricator-request
Pike
: review+
Details | Review
46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
gps
: review+
Details | Review
46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
aki
: review+
Details | Review
46 bytes, text/x-phabricator-request
bhearsum
: review+
Details | Review
08:22 <glandium> nthomas: https://bugzilla.mozilla.org/show_bug.cgi?id=1277905#c11
08:25 <nthomas> I thought that was known limitiation, certainly didn't make any bones about it in the bug
08:26 <nthomas> aka it was set up because people wanted to hack on l10n, so why wait for an en-US build every time ?
08:26 <glandium> nthomas: because there are many ways this can fail without involving any change in l10n code
08:27 <glandium> like, push a tree a little old, boom
08:28 <nthomas> this is the problem with l10n, there a goalposts, let alone any that sit still for a moment
08:28 <nthomas> *are no goalposts
08:30 <glandium> one of the very basic problems of those builds (and here I'm speaking generally, not just the try ones), is that they download the last nightly. Which means if you retrigger a l10n nightly job from 2 days ago, it will do l10n repacks of the last nightly
08:31 <glandium> why those builds are not changed to pull the build result from the current push is beyond me
08:31 <nthomas> you know this is configurable, right ?
08:31 <glandium> nthomas: on the nightly jobs?
08:32 <nthomas> no, on the try job
08:32 <nthomas> https://wiki.mozilla.org/ReleaseEngineering/TryServer#Desktop_l10n_jobs
08:32 <glandium> I'm speaking generally
08:33 <nthomas> the nightly jobs are now triggered by the en-US with a specific directory (bug 1250629), rebuilding works as you desire for them
08:34 <glandium> sounds to me this should be the default behavior for try
08:37 <glandium> the point is, it's not nice that people not touching l10n code are getting build failures because of that
08:38 <glandium> people iterating on l10n code can do the required changes to not depend on a build to finish to iterate quickly
08:40 <nthomas> that's sounds do-able, file a bug and ask catlee to triage ?
Flags: needinfo?(catlee)
From 30k ft, the solution here is to make a change like

- "en_us_binary_url": "http://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central",
+ "en_us_binary_url": "http://archive.mozilla.org/pub/firefox/try-builds/%(who)s-%(revision)s/try-%(platform)s",

at https://dxr.mozilla.org/mozilla-central/rev/214884d507ee369c1cf14edb26527c4f9a97bf48/testing/mozharness/configs/single_locale/try.py#4. We're not interpolating that config variable at the moment, so we'd need to make sure all those parameters were available and used.
and the l10n build would need to depend on the try "normal" build being finished
I'll let catlee do the triage on how-important, but this was one of my "Make L10n Great Again" 2016 campaign promises ;-) With the following specific plan:

* Make the existing try job a "L10n(Quick)" which is based off the last Nightly (currently to be m-c, but eventually might have logic to identify which tree the code came from) -- and passes only 2 or 3 [hardcoded] locales to its config.
* Make a corresponding L10n(Short) job that is essentially the same thing as existing, but based on the current en-US build that was done on the push (and run it more often, on more CI than just try, based on what files changed)
* Make a corresponding L10n(1..6) that is literally what we run on nightlies incase someone actually needs to test changes to all-locales logic and the like.

The last piece is *unlikely* to be existing in Buildbot code, I'm not even sure if I'd get the second one done for buildbot (due to the dep stuff), but either way, that's my rough intent atm. (this is also the first place I've articulated that)
I have to say that I've not discovered the use case for the "quick" job. 

As far as I make things up, mozharness resets mozilla-central to the version of the build it downloaded, right? So at the point where we're repacking, we're either discovering bugs of inconsistent source, or testing the source code of the nightly.

Right?

Also, I think Short and 1..6 are the same thing. The only difference I could spot between the two would be the all-locales list, and the current logic doesn't change that?
(In reply to Axel Hecht [:Pike] from comment #4)
> As far as I make things up, mozharness resets mozilla-central to the version
> of the build it downloaded, right? So at the point where we're repacking,
> we're either discovering bugs of inconsistent source, or testing the source
> code of the nightly.

That's covered by http://hg.mozilla.org/mozilla-central/file/default/testing/mozharness/scripts/desktop_l10n.py#l655 and setting update_gecko_source_to_enUS to False for try.
The original intent was to provide a way to test l10n jobs in try - and we have that now, yay!

I do understand the desire to have the L10n(short) job which depends on the try en-US build. Unless this is really simple to implement in buildbot, I think we should wait until Try scheduling is all migrated over to TC.
Flags: needinfo?(catlee)
Component: General Automation → General
Ok, I'm now morphing this one.

L10n is now all in TC

We'll create an L10n job testing a small few locales, that then can run on all trees.

Caveat: Will not yet run on trees that only do 'N' Jobs on-push (that will be followup work, possibly as part of 'shippable builds')

This will fix the Try L10n issues people have been seeing and make us have some sanity on m-c pushes that could break l10n.
Summary: L10n try jobs should build off the corresponding try build by default → L10n jobs should run per-push based on the corresponding builds
for L10n jobs should run per-push based on the corresponding builds

MozReview-Commit-ID: 9d9RsjLVmPj
for L10n jobs should run per-push based on the corresponding builds

MozReview-Commit-ID: 59psFKNDxLr
for L10n jobs should run per-push based on the corresponding builds

MozReview-Commit-ID: FIIR7rdzFgZ
for L10n jobs should run per-push based on the corresponding builds

MozReview-Commit-ID: 4NLL8Z83Q40
for L10n jobs should run per-push based on the corresponding builds

MozReview-Commit-ID: JSj0uZOuFGg
for L10n jobs should run per-push based on the corresponding builds

MozReview-Commit-ID: HFRnU3aEQOz
for L10n jobs should run per-push based on the corresponding builds
for L10n jobs should run per-push based on the corresponding builds

MozReview-Commit-ID: 6BBVO8x2rtm
for L10n jobs should run per-push based on the corresponding builds
for L10n jobs should run per-push based on the corresponding builds
for L10n jobs should run per-push based on the corresponding builds
for L10n jobs should run per-push based on the corresponding builds
Comment on attachment 8980722 [details]
Bug 1286092 - Actually enable l10n repacks based on the same push on-change. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1409
Attachment #8980722 - Flags: review+
Comment on attachment 8980709 [details]
Bug 1286092 - Fix mistake with build_date in index routes. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1398
Attachment #8980709 - Flags: review+
Comment on attachment 8980710 [details]
Bug 1286092 - Stop using old style l10n routes for nightly. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1399
Attachment #8980710 - Flags: review+
Comment on attachment 8980711 [details]
Bug 1286092 - Add a pushlog-id format for l10n on-change routes. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1400
Attachment #8980711 - Flags: review+
Comment on attachment 8980715 [details]
Bug 1286092 - Make taskgraph sparse checkout pull the onchange files. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1403
Attachment #8980715 - Flags: review+
Comment on attachment 8980716 [details]
Bug 1286092 - Add signing for linux/mac, needed for on-change l10n. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1404
Attachment #8980716 - Flags: review+
Comment on attachment 8980717 [details]
Bug 1286092 - Do repackage for platforms to support on-change l10n. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1405
Attachment #8980717 - Flags: review+
Comment on attachment 8980718 [details]
Bug 1286092 - Stub installer attribute. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1406
Attachment #8980718 - Flags: review+
Comment on attachment 8980721 [details]
Bug 1286092 - Do repackage-signing on-change for on-change l10n. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1408
Attachment #8980721 - Flags: review+
Comment on attachment 8980720 [details]
Bug 1286092 - Enforce USE_STUB_INSTALLER when expecting to build the stub. r=gps

Gregory Szorc [:gps] has approved the revision.

https://phabricator.services.mozilla.com/D1407
Attachment #8980720 - Flags: review+
Comment on attachment 8980712 [details]
Bug 1286092 - Explicitly specify locales to use for on-change l10n. r=Pike

Axel Hecht [:Pike] has approved the revision.

https://phabricator.services.mozilla.com/D1401
Attachment #8980712 - Flags: review+
Comment on attachment 8980713 [details]
Bug 1286092 - Explicitly specify locales to use for on-change l10n for mobile. r=Pike

Axel Hecht [:Pike] has approved the revision.

https://phabricator.services.mozilla.com/D1402
Attachment #8980713 - Flags: review+
for L10n jobs should run per-push based on the corresponding builds
for L10n jobs should run per-push based on the corresponding builds
Comment on attachment 8981289 [details]
Bug 1286092 - Don't ridealong l10n anymore, now that we're going to be per-change anyway. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1436
Attachment #8981289 - Flags: review+
Comment on attachment 8981494 [details]
Bug 1286092 - Make l10n onpush tier 1. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1450
Attachment #8981494 - Flags: review+
Comment on attachment 8981493 [details]
Bug 1286092 - Enable desktop l10n on-push for beta tasks too. r=aki

Aki Sasaki [:aki] has approved the revision.

https://phabricator.services.mozilla.com/D1449
Attachment #8981493 - Flags: review+
Pushed by Callek@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3c05f6e6a57
Fix mistake with build_date in index routes. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/3eb4e39b9bcd
Stop using old style l10n routes for nightly. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/6eb40b38f9c9
Add a pushlog-id format for l10n on-change routes. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/db19ea492b02
Explicitly specify locales to use for on-change l10n. r=Pike
https://hg.mozilla.org/integration/mozilla-inbound/rev/556f54c5e209
Explicitly specify locales to use for on-change l10n for mobile. r=Pike
https://hg.mozilla.org/integration/mozilla-inbound/rev/5617053b1740
Make taskgraph sparse checkout pull the onchange files. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b828445f551
Add signing for linux/mac, needed for on-change l10n. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2271cd6af26
Do repackage for platforms to support on-change l10n. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc6aebca89e3
Stub installer attribute. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa3e38422d49
Enforce USE_STUB_INSTALLER when expecting to build the stub. r=gps
https://hg.mozilla.org/integration/mozilla-inbound/rev/878e174255ff
Do repackage-signing on-change for on-change l10n. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d68dfd6325c
Don't ridealong l10n anymore, now that we're going to be per-change anyway. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8c01d976acd
Actually enable l10n repacks based on the same push on-change. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9cf945aa4f9
Enable desktop l10n on-push for beta tasks too. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/57a592e33f36
Make l10n onpush tier 1. r=aki
Backout by btara@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef828badfd93
Backed out changeset 57a592e33f36 for L10n bustages CLOSED TREE
Backout by btara@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eae35fb53eb8
Backed out 16 changesets for L10n bustages CLOSED TREE
for L10n jobs should run per-push based on the corresponding builds
Please also take a look at this failure on the first landing if it's related: https://treeherder.mozilla.org/logviewer.html#?job_id=180897684&repo=mozilla-inbound
Flags: needinfo?(bugspam.Callek)
Comment on attachment 8981846 [details]
Bug 1286092 - Add missing inbound and autoland configs. r=bhearsum

Ben Hearsum (:bhearsum) has approved the revision.

https://phabricator.services.mozilla.com/D1468
Attachment #8981846 - Flags: review+
Pushed by Callek@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/361cba5c46e6
Fix mistake with build_date in index routes. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6cb27d480dc
Stop using old style l10n routes for nightly. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/48daf41a985e
Add a pushlog-id format for l10n on-change routes. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2f587986a3b
Explicitly specify locales to use for on-change l10n. r=Pike
https://hg.mozilla.org/integration/mozilla-inbound/rev/ddfda72d1d31
Explicitly specify locales to use for on-change l10n for mobile. r=Pike
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd818612e03c
Make taskgraph sparse checkout pull the onchange files. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/496377072757
Add signing for linux/mac, needed for on-change l10n. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3405a0082d6
Do repackage for platforms to support on-change l10n. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/e250856b4688
Stub installer attribute. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/a562a809e8dc
Enforce USE_STUB_INSTALLER when expecting to build the stub. r=gps
https://hg.mozilla.org/integration/mozilla-inbound/rev/e73477aafd06
Do repackage-signing on-change for on-change l10n. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a9c609fa789
Don't ridealong l10n anymore, now that we're going to be per-change anyway. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/698fca98c32e
Actually enable l10n repacks based on the same push on-change. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/37d0408ce6e3
Enable desktop l10n on-push for beta tasks too. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7472bf663bd
Make l10n onpush tier 1. r=aki
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8b1587aac9d
Add missing inbound and autoland configs. r=bhearsum
Assignee: nobody → bugspam.Callek
Depends on: 1465799
See Also: → 1465836
Could this be the cause of the broken code coverage build (failing in l10n-check)?
https://treeherder.mozilla.org/logviewer.html#?job_id=181226998&repo=mozilla-central
(In reply to Marco Castelluccio [:marco] from comment #46)
> Could this be the cause of the broken code coverage build (failing in
> l10n-check)?
> https://treeherder.mozilla.org/logviewer.html#?job_id=181226998&repo=mozilla-
> central

Nope, the underlying issue there is [task 2018-05-31T22:34:03.902Z] 22:34:03     INFO -  l10n-check> rsync: write failed on "/builds/worker/workspace/build/src/obj-firefox/dist/l10n-stage/firefox/libxul.so": No space left on device (28)
Flags: needinfo?(bugspam.Callek)
(In reply to Justin Wood (:Callek) from comment #47)
> (In reply to Marco Castelluccio [:marco] from comment #46)
> > Could this be the cause of the broken code coverage build (failing in
> > l10n-check)?
> > https://treeherder.mozilla.org/logviewer.html#?job_id=181226998&repo=mozilla-
> > central
> 
> Nope, the underlying issue there is [task 2018-05-31T22:34:03.902Z] 22:34:03
> INFO -  l10n-check> rsync: write failed on
> "/builds/worker/workspace/build/src/obj-firefox/dist/l10n-stage/firefox/
> libxul.so": No space left on device (28)

Yeah, I thought this patch might somehow have made l10n-check run when it wasn't previously, but it sounds like it isn't the case.
We're probably just going to disable l10n-check for ccov builds in bug 1465996.
You need to log in before you can comment on or make changes to this bug.