Closed Bug 1437695 Opened 2 years ago Closed 2 years ago

Add temporary "stylo-only" build job

Categories

(Firefox Build System :: General, enhancement)

x86_64
Linux
enhancement
Not set

Tracking

(firefox-esr52 unaffected, firefox58 wontfix, firefox59 wontfix, firefox60 fixed)

VERIFIED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: cpeterson, Assigned: kmoir)

References

Details

(Whiteboard: [stylo:p1])

Attachments

(1 file, 3 obsolete files)

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
Stupid question: why not just build normal linux builds with --enable-stylo=only? (or even on all platforms, except android)
(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...
Adding a build-only variation is fine: it's tests that contribute to our scaling/cost concerns. So this is a very reasonable request.
cpeterson: In what timeframe are you looking for this to be implemented?
Flags: needinfo?(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. :)
Flags: needinfo?(cpeterson)
Assignee: nobody → kmoir
Hi Kim, do you have any updates on ETA?
Flags: needinfo?(kmoir)
I'm testing the patches on try but do not have a successful build yet.
Flags: needinfo?(kmoir)
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.
I believe it is my build config, not the build itself. I'll attach my patches so far.
Attached patch wip patches (obsolete) — Splinter Review
Will look at it again in the morning.
Oh, okay. If there is something break the build itself, please ni? me.
Attached patch bug1437695.patch (obsolete) — Splinter Review
Attachment #8952604 - Attachment is obsolete: true
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
Flags: needinfo?(xidorn+moz)
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.
Flags: needinfo?(xidorn+moz)
Depends on: 1440141
With bug 1440141 fixed, stylo-only build should work again. ni? me if there is still any problem.
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.
Flags: needinfo?(xidorn+moz)
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).
Flags: needinfo?(xidorn+moz)
Attachment #8953061 - Flags: review?(core-build-config-reviews)
Attachment #8953061 - Flags: feedback?(sheehan)
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...
Attachment #8953061 - Flags: review?(core-build-config-reviews)
Attachment #8954111 - Flags: review?(nfroyd)
Comment on attachment 8954111 [details] [diff] [review]
bug1437695-new.patch

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

Thank you!
Attachment #8954111 - Flags: review?(nfroyd) → review+
Pushed by kmoir@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8e5b45eedde
Add temporary stylo-only build job r=nfroyd
https://hg.mozilla.org/mozilla-central/rev/a8e5b45eedde
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
The "linux-stylo-only" opt/debug build jobs are now running and green. Thanks, Kim!
Status: RESOLVED → VERIFIED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.