1330666 - enable linux64-stylo builds

Status: RESOLVED FIXED

(Infrastructure & Operations Graveyard :: CIDuty, task, P2)

Description

[Kim Moir [:kmoir] ET]

email from cpeterson
--
hi Kim, I just wanted to give you a heads up that, in a couple weeks, the Quantum Style team would like to add a new build config (if that is the correct term?) to Treeherder running on mozilla-central (and inbound and autoland) pushes. We would want this temporary build config ("linux64-stylo") to run the usual build and test jobs (with some Stylo flags enabled). We could then retire the tests running in the incubator/stylo repo.

We're tracking the dependencies in meta bug 1330414 (alias "stylo-central"): Run linux64-stylo tests in mozilla-central.

https://bugzilla.mozilla.org/showdependencytree.cgi?id=1330414&hide_resolved=1

--

questions from me

Is there a mozconfig concurrently defined?
Is branding different?
please define list of tests that need to run
talos required or not?
Does this build need to run on every commit or could it run periodically (to reduce AWS costs)

Comment 1

[Kim Moir [:kmoir] ET]

Another question, will these additional builds + tests ride the trains and be uplifted to aurora or will the preferences just be merged into regular firefox builds at some point and we'll be able to disable the style specific builds and tests.

Comment 2

[Bobby Holley (:bholley)]

(In reply to Kim Moir [:kmoir] from comment #1)
> Another question, will these additional builds + tests ride the trains and
> be uplifted to aurora or will the preferences just be merged into regular
> firefox builds at some point and we'll be able to disable the style specific
> builds and tests.

I don't think we've fully sorted that out yet. Does it have a significant impact on what we do now?

At present, the aim for this configuration is just to get the stylo builds green on CI (incrementally turning more tests on), and then flip the switch on Nightly. Once that's happened, the proposed linux64-stylo configuration will have served its purpose.

After that, we'll need to sort out exactly what kind of rollout we want for Quantum, in particular whether all the quantum projects ride the trains together, and what kind of CI we want to keep running on non-quantum builds in case we need to flip parts of quantum off.

Ideally we could punt on that discussion until later. But if there are reasons why it affects our decision-making now, I'd like to understand them.

Comment 3

[Chris Peterson [:cpeterson]]

(In reply to Kim Moir [:kmoir] from comment #1)
> Another question, will these additional builds + tests ride the trains and
> be uplifted to aurora or will the preferences just be merged into regular
> firefox builds at some point and we'll be able to disable the style specific
> builds and tests.

We won't need these linux64-stylo builds + tests to ride the trains.

When Stylo is ready to ride the trains, it will be enabled in regular Firefox builds (perhaps in just one one platform initially to test the waters). At that time, we might want a "linux64-nostylo" or whatever build config to ride the trains to ensure we have don't break the non-Stylo code, in case we need to disable Stylo at the last minute. We did something similar for e10s.

Comment 4

[Kim Moir [:kmoir] ET]

> I don't think we've fully sorted that out yet. Does it have a significant impact on what we do now?

No, that's fine, thanks

> from email from cpeterson

    Does this build need to run on every commit or could it run periodically (to reduce AWS costs)

As a first step, I think we can just focus on running it on m-c, which should make the AWS cost a non issue. We can weight the rest of the tradeoffs later (unless we need to now for budgeting reasons).


kmoir> Thanks this is good to know, I will chat with sheriffs to see if they see any issues with this (they probably don't want failures on this build on merges if this code is not exercised on m-i etc)

Comment 5

[Bobby Holley (:bholley)]

(In reply to Kim Moir [:kmoir] from comment #4)
> kmoir> Thanks this is good to know, I will chat with sheriffs to see if they
> see any issues with this (they probably don't want failures on this build on
> merges if this code is not exercised on m-i etc)

In the first phase I'm talking about, the builds would be hidden by default. We would certainly need the builds on inbound if they were going to be tier 1.

Comment 6

[Kim Moir [:kmoir] ET]

So to consolidate the list of requirements
this is a tier 2 build 
will be initially hidden on m-c only
no branding changes
once it is now longer required and code is merged into the regular nightly it will be deleted
perhaps a need for a no-stylo build at this time, to verify that tests still work there
small set of tests initially, this will expand
mozonfig is the same as the existing linux64 build but with --enable-stylo

Comment 7

[Bobby Holley (:bholley)]

(In reply to Kim Moir [:kmoir] from comment #6)
> So to consolidate the list of requirements
> this is a tier 2 build
> will be initially hidden on m-c only
> no branding changes
> once it is now longer required and code is merged into the regular nightly
> it will be deleted
> perhaps a need for a no-stylo build at this time, to verify that tests still
> work there
> small set of tests initially, this will expand
> mozonfig is the same as the existing linux64 build but with --enable-stylo

That all sounds right.

Comment 8

[Kim Moir [:kmoir] ET]

Attachment: wip patch

Comment 9

[Kim Moir [:kmoir] ET]

Attachment: stylo-2.patch

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

task graph with builds and tests enabled
https://public-artifacts.taskcluster.net/b65JrIfvTU6jTwiQuJ-v0w/0/public/full-task-graph.json

I'll setup a hook or use dustin's new in tree cron stuff once the other dependent bugs are resolved

Comment 10

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 8828883 [details] [diff] [review]
stylo-2.patch

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

::: taskcluster/ci/build/linux.yml
@@ +354,5 @@
> +        product: firefox
> +        job-name: linux64-stylo-opt
> +    treeherder:
> +        platform: linux64/stylo
> +        symbol: tc(Bo)

Not sure I'm right here, but isn't this conflicting with ASan `Bo` builds symbols in the tree?

@@ +380,5 @@
> +        product: firefox
> +        job-name: linux64-stylo-debug
> +    treeherder:
> +        platform: linux64/stylo
> +        symbol: tc(Bd)

Isn't this conflicting with ASan `Bd` builds symbols in the tree?

Comment 11

[Kim Moir [:kmoir] ET]

Attachment: stylo-3.patch

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f05517eee951
enable linux64-stylo builds r=mtabara

Comment 13

[Gregory Szorc [:gps]]

There are now 2 linux64-stylo/opt and linux64-stylo/debug sections in taskcluster/ci/build/linux.yml. These need to be reconciled. Needinfo kmoir for that.

Also, needinfo dustin so he is aware that duplicate keys can exist in YAML files. I'm, uh, kinda surprised that's not a parse error in YAML. IMO it should be caught in the taskgraph layer.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff553ea5315a
enable linux64-stylo builds r=kmoir

Comment 15

[Kim Moir [:kmoir] ET]

I just removed the extra stanzas

Comment 16

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

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

Comment 17

[Phil Ringnalda (:philor)]

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

Comment 18

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

I have noticed that the Python YAML parser doesn't have an issue with duplicate keys, although the JS parser we use does.  Without regexes or a different parser, I don't know how we could catch that, sadly.

Comment 19

[Kim Moir [:kmoir] ET]

reopening because I need to enable the tc hook once the issues in bug 1322769 are resolved

Comment 20

[Cameron McCormack (:heycam)]

I guess we are close to working on mozilla-central, but it looks like this patch has stopped the reftest-stylo jobs running on the stylo repo.  Can we have them continue running until we're ready to move over fully?  https://treeherder.mozilla.org/#/jobs?repo=stylo&revision=f8e2a5d85f4777a43bf8cf2c50ee04e02eb37ae7&filter-searchStr=linux64-stylo

Comment 21

[Kim Moir [:kmoir] ET]

okay, I'm investigating a patch for that now

Comment 22

[Kim Moir [:kmoir] ET]

Attachment: stylo3.patch

I think this will fix it

Comment 23

[Cameron McCormack (:heycam)]

Thanks Kim.  I've landed that on the stylo repo now.

Comment 24

[Kim Moir [:kmoir] ET]

Going to close this even though the talos dependant bug 1338871  is not complete, it has still been assigned to me