Closed Bug 1437695 Opened 6 years ago Closed 6 years ago

Add temporary "stylo-only" build job


(Firefox Build System :: General, enhancement)

Not set


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

Tracking Status
firefox-esr52 --- unaffected
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- fixed


(Reporter: cpeterson, Assigned: kmoir)



(Whiteboard: [stylo:p1])


(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:
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

But the build fails with this error so not sure if there is something wrong with my config or something else
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]

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/
@@ +447,5 @@
>          'debug-artifact': 'builds/releng_sub_%s_configs/',
>          'devedition': 'builds/releng_sub_%s_configs/',
>          'dmd': 'builds/releng_sub_%s_configs/',
> +        'stylo': 'builds/releng_sub_%s_config/',
> +        'stylo-debug': 'builds/releng_sub_%s_config/'

Where are these .py files coming from?  `find . -name \*` 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]

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

Thank you!
Attachment #8954111 - Flags: review?(nfroyd) → review+
Pushed by
Add temporary stylo-only build job r=nfroyd
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
The "linux-stylo-only" opt/debug build jobs are now running and green. Thanks, Kim!
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.