1437695 - Add temporary "stylo-only" build job

Status: VERIFIED FIXED

(Firefox Build System :: General, enhancement)

Description

[Chris Peterson [:cpeterson]]

The Stylo team would like to add a new, temporary build job called something like "stylo-only".

Xidorn landed a configure option (--enable-stylo=only in bug 1430014) to compile Firefox without Gecko's old style system code (using #ifdef MOZ_OLD_STYLE). Until we can actually delete the old code, we'd like a new build job to ensure we don't break the future build without the old code. We can remove this "stylo-only" build job after we remove the old code (probably in Nightly 61).

We would like to:

* build opt and debug Linux64 with --enable-stylo=only
* on mozilla-central and try branch commits
* We don't need to run any tests. We just want to check for build breaks.

Here is a link to the usage message for --enable-stylo:

https://searchfox.org/mozilla-central/rev/a5abf843f8fac8530aa5de6fb40e16547bb4a47a/toolkit/moz.configure#555-588

Comment 1

[Mike Hommey [:glandium]]

Stupid question: why not just build normal linux builds with --enable-stylo=only? (or even on all platforms, except android)

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Mike Hommey [:glandium] from comment #1)
> Stupid question: why not just build normal linux builds with
> --enable-stylo=only? (or even on all platforms, except android)

I think two reasons:
1. stylo-chrome hasn't been enabled, and using --enable-stylo=only would unexpectedly enable stylo-chrome.
2. android doesn't have as much test coverage as other platforms, so we still want to run stylo-disabled tests on linux64.

But I guess those are getting fixed soon...

Comment 3

[Gregory Szorc [:gps]]

Adding a build-only variation is fine: it's tests that contribute to our scaling/cost concerns. So this is a very reasonable request.

Comment 4

[Kim Moir [:kmoir] ET]

cpeterson: In what timeframe are you looking for this to be implemented?

Comment 5

[Chris Peterson [:cpeterson]]

(In reply to Kim Moir [:kmoir] ET from comment #4)
> cpeterson: In what timeframe are you looking for this to be implemented?

Some time in the next week or two would be good. This is not a critical priority, but sooner is always appreciated. :)

Comment 6

[Chris Peterson [:cpeterson]]

Hi Kim, do you have any updates on ETA?

Comment 7

[Kim Moir [:kmoir] ET]

I'm testing the patches on try but do not have a successful build yet.

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

If you're hitting build failures, it makes sense to file bug about what is failing.

And if that has been broken, we probably should have it run on autoland as well.

Comment 9

[Kim Moir [:kmoir] ET]

I believe it is my build config, not the build itself. I'll attach my patches so far.

Comment 10

[Kim Moir [:kmoir] ET]

Attachment: wip patches

Will look at it again in the morning.

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

Oh, okay. If there is something break the build itself, please ni? me.

Comment 12

[Kim Moir [:kmoir] ET]

Attachment: bug1437695.patch

Comment 13

[Kim Moir [:kmoir] ET]

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

But the build fails with this error so not sure if there is something wrong with my config or something else

https://treeherder.mozilla.org/logviewer.html#?job_id=163531572&repo=try&lineNumber=19400

Comment 14

[Xidorn Quan [:xidorn] UTC+11]

So it is indeed that the build was broken, by bug 1438020. We need to fix it, and probably should enable those builds on autoland and mozilla-inbound as well given this situation. Bug 1438020 shows a case that it can be broken trivially by an innocent change which people may not even care to run a try push at all.

Comment 15

[Xidorn Quan [:xidorn] UTC+11]

With bug 1440141 fixed, stylo-only build should work again. ni? me if there is still any problem.

Comment 16

[Kim Moir [:kmoir] ET]

Attachment: bug1437695.patch

new try run here

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

Comment 17

[Kim Moir [:kmoir] ET]

The build is green on the try push, could you verify that it is what you expect as I don't have any insight into that.

Comment 18

[Xidorn Quan [:xidorn] UTC+11]

Yeah, I think it is what I expect. If you used the same configuration between your try run in comment 13 and comment 16, I think that's enough to tell that it works as expected (since it caught the breakage of the build in comment 13).

Comment 19

[Nathan Froyd [:froydnj]]

Comment on attachment 8953061 [details] [diff] [review]
bug1437695.patch

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

Nit: Do we want to put stylo-only into the names?  So the mozconfigs would be stylo-only/stylo-only-debug, the task name would have "only"/"Only" mentioned somewhere, and so forth.

Canceling because of the confusion noted below, otherwise this looks fine.

::: testing/mozharness/mozharness/mozilla/building/buildbase.py
@@ +447,5 @@
>          'debug-artifact': 'builds/releng_sub_%s_configs/%s_debug_artifact.py',
>          'devedition': 'builds/releng_sub_%s_configs/%s_devedition.py',
>          'dmd': 'builds/releng_sub_%s_configs/%s_dmd.py',
> +        'stylo': 'builds/releng_sub_%s_config/%s_stylo.py',
> +        'stylo-debug': 'builds/releng_sub_%s_config/%s_stylo_debug.py'

Where are these .py files coming from?  `find . -name \*stylo.py` on inbound doesn't show these files, but your try push shows that things are building, so I am confused...

Comment 20

[Kim Moir [:kmoir] ET]

Attachment: bug1437695-new.patch

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

Comment 21

[Nathan Froyd [:froydnj]]

Comment on attachment 8954111 [details] [diff] [review]
bug1437695-new.patch

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

Thank you!

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8e5b45eedde
Add temporary stylo-only build job r=nfroyd

Comment 23

[Arthur Iakab [arthur_iakab]]

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

Comment 24

[Chris Peterson [:cpeterson]]

The "linux-stylo-only" opt/debug build jobs are now running and green. Thanks, Kim!