Drop support for universal Mac builds in Firefox 53

RESOLVED FIXED in Firefox 53

Status

()

Core
Build Config
RESOLVED FIXED
a year ago
5 months ago

People

(Reporter: gps, Assigned: mshal)

Tracking

unspecified
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(relnote-firefox 53+, firefox53 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

a year ago
Per bug 1183613 comment 16, we can drop universal mac builds in Firefox 53.

mozilla-central becomes Firefox 53 on November 7.
The question is: how far do we want to go with dropping universal mac builds. There are several level:
- Stop building them
- Remove the corresponding mozconfigs
- Remove the corresponding packaging code

The first only has an impact on Firefox.
The second might have an impact on third-parties relying on those mozconfigs to build their own universal builds (I'm thinking comm-central and other third-parties like bluegriffon)
The third would remove any support for third-parties to build their own universal builds.
Daniel, I remember we talked about universal builds some time ago, I don't remember if you'd be bothered if they were dropped entirely.
Flags: needinfo?(daniel)
(Reporter)

Comment 3

a year ago
Q: do Gecko/Firefox developers have to spend time making OS X code work on 32-bit Macs (excluding general 32-bit compatibility for other platforms like Windows and Linux)? If so, then I'm inclined to say we should drop all support for 32-bit Mac compatibility so developers don't need to worry about it.
I think that we should first just switch the existing universal builds to build single-architecture x86-64 opt builds, leaving all the build machinery in place for a full release cycle. Assuming that nothing goes awry, in the next release cycle we should start removing the support code (including mozconfigs, etc).
(Reporter)

Comment 5

9 months ago
bsmedberg: can we proceed with this?
Flags: needinfo?(benjamin)
I don't know why but I never received the earlier needinfo...

I have no problem with step 1, of course, and I think I am ok with step 2. BlueGriffon still has OS X 32bits users but they're vanishing at rather fast pace. I can live with that negligible loss.

Mike, thanks a lot for asking.
Flags: needinfo?(daniel)

Comment 7

9 months ago
Yes gogogo!
Flags: needinfo?(benjamin)
(Reporter)

Comment 8

9 months ago
I assume comment #7 means we can turn off universal builds in automation. So let's proceed with that.

bsmedberg: can you please clarify how far we should go with dropping in-tree *support* for universal builds (per comment #1)?
Flags: needinfo?(benjamin)
(Reporter)

Comment 9

9 months ago
rail: do any balrog changes need to be made before we turn off universal os x builds? If so, could you please get bugs on file and have them block this bug?
Flags: needinfo?(rail)
302 jlund|releaseduty
Flags: needinfo?(rail) → needinfo?(jlund)

Comment 11

9 months ago
I believe we should feel free to remove build support for universal builds, if doing so would make your life easier. I'm sure that soon after we stop building UB in automation, things will start to break.

I think we should attempt to keep Mac x86 building, as a tier-3 config maintained by the community, if somebody steps forward to periodically build/maintain it.
Flags: needinfo?(benjamin)
So, funny thing I just noticed in bug 1321065: browser/config/mozconfigs/macosx64/nightly and browser/config/mozconfigs/macosx-universal/nightly are different. Whoever picks this bug should look into this.
(In reply to Mike Hommey [:glandium] from comment #12)
> So, funny thing I just noticed in bug 1321065:
> browser/config/mozconfigs/macosx64/nightly and
> browser/config/mozconfigs/macosx-universal/nightly are different. Whoever
> picks this bug should look into this.

I would guess it's just "someone forgot to update the other mozconfig that wasn't actually used for anything currently".
(In reply to Benjamin Smedberg [:bsmedberg] from comment #11)
> I think we should attempt to keep Mac x86 building, as a tier-3 config
> maintained by the community, if somebody steps forward to periodically
> build/maintain it.

Only if it's the cross-compiled variant. We're not keeping old Mac build hardware around for a tier 3 project.
(In reply to Gregory Szorc [:gps] from comment #9)
> rail: do any balrog changes need to be made before we turn off universal os
> x builds? If so, could you please get bugs on file and have them block this
> bug?

We haven't tested updating a nightly user from universal -> 64-bit opt. We should do that.
(In reply to Chris Cooper [:coop] from comment #15) 
> We haven't tested updating a nightly user from universal -> 64-bit opt. We
> should do that.

This is bug 1310849.
Depends on: 1310849
(Assignee)

Comment 17

9 months ago
I have some of this ready to go - I just need to sort out the mozconfigs (in addition to the nightly differences, we need beta & release configs).
Assignee: nobody → mshal
(In reply to Chris Cooper [:coop] from comment #14)
> Only if it's the cross-compiled variant. We're not keeping old Mac build
> hardware around for a tier 3 project.

FWIW, Tier 3 has always meant "we don't run CI, but we'll take patches to keep it working".
Comment hidden (mozreview-request)
(Reporter)

Comment 20

9 months ago
If x86 Mac is tier 3 and the only thing we want to support is that x86 Mac users can still run Firefox, in my opinion as build module owner, that means universal builds no longer serve a useful function and all code and functionality related to universal builds can be removed.

If universal build support were easy to keep around, I'd say "why not." But the reality is the code for supporting universal builds is pretty messy and it gets in the way of various refactorings. It's in the best interest of build system maintainers for that code and associated complexity to disappear.
(Reporter)

Comment 21

9 months ago
mozreview-review
Comment on attachment 8815878 [details]
Bug 1295375 - Drop universal Mac builds;

https://reviewboard.mozilla.org/r/96662/#review96918

This is mostly good (and exciting!). Just a VCS nit and a question about the nightly config file, which contained some non-obvious changes.

::: browser/config/mozconfigs/macosx64/beta:1
(Diff revision 1)
> +MOZ_AUTOMATION_SDK=${MOZ_AUTOMATION_SDK-1}

Please `hg copy` these files from the macosx-universal versions so VCS history is preserved.

::: browser/config/mozconfigs/macosx64/nightly:1
(Diff revision 1)
> -. $topsrcdir/build/macosx/mozconfig.common
> +. "$topsrcdir/browser/config/mozconfigs/macosx64/common-opt"

I'm curious why this change was made. mozconfig.common handles cross vs native builds. This change seems unrelated to disabling universal builds?
Attachment #8815878 - Flags: review?(gps) → review-
(Assignee)

Comment 22

9 months ago
mozreview-review
Comment on attachment 8815878 [details]
Bug 1295375 - Drop universal Mac builds;

https://reviewboard.mozilla.org/r/96662/#review97258

::: browser/config/mozconfigs/macosx64/nightly:1
(Diff revision 1)
> -. $topsrcdir/build/macosx/mozconfig.common
> +. "$topsrcdir/browser/config/mozconfigs/macosx64/common-opt"

Sorry, I should have explained the changes to the nightly config. In macosx-universal, the common-opt file shared some config among the nightly/beta/release mozconfigs. In macosx64, we only had nightly, so some of the things that were in common-opt were just added directly to the nightly config. For example, the universal common-opt file specifies MOZ_TELEMETRY_REPORTING=1, but in macosx64 this was put into the nightly config.

But now we have beta & release mozconfigs in macosx64, so it makes sense to introduce the common-opt file there. So the nightly mozconfig changed since I removed the lines that are duplicated with common-opt. Including common-opt also includes build/macosx/mozconfig.common, so the cross-build logic is still intact.

It should be a bit more clear with the 'hg copy' for beta & release, and if you diff the new nightly config with the macosx-universal/nightly config, you'll see they are identical save for the common-opt include.
Comment hidden (mozreview-request)
(Reporter)

Comment 24

9 months ago
mozreview-review
Comment on attachment 8815878 [details]
Bug 1295375 - Drop universal Mac builds;

https://reviewboard.mozilla.org/r/96662/#review97548

Good riddance.
Attachment #8815878 - Flags: review?(gps) → review+
(Assignee)

Comment 25

9 months ago
Hmm, it seems the Wr and W(1) test suites (as well as the e10s counterparts) are failing. ted: do any of these look obvious to you? I'm not sure what I'm missing:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce0d646eb244aca032fbff8e8e0edaab1548b55b
Flags: needinfo?(ted)

Comment 26

9 months ago
Wr looks like a TEST-UNEXPECTED-PASS, which seems like a win in my book ;-) You probably just need to update testing/web-platform/meta/mathml/relations/css-styling/color-1.html.ini because the hardcoding of specific windows and mac versions in there is ugly-ass-terrible

http://searchfox.org/mozilla-central/source/testing/web-platform/meta/encrypted-media/clearkey-mp4-playback-temporary-encrypted-clear.html.ini#1 seems to be in a similarly-terrible state for W1
Flags: needinfo?(ted)

Comment 27

9 months ago
mozreview-review
Comment on attachment 8815878 [details]
Bug 1295375 - Drop universal Mac builds;

https://reviewboard.mozilla.org/r/96662/#review97736

::: testing/mozharness/configs/merge_day/aurora_to_beta.py:39
(Diff revision 2)
>           "ac_add_options --enable-official-branding")
>          for f in ["browser/config/mozconfigs/linux32/l10n-mozconfig",
>                    "browser/config/mozconfigs/linux64/l10n-mozconfig",
>                    "browser/config/mozconfigs/win32/l10n-mozconfig",
>                    "browser/config/mozconfigs/win64/l10n-mozconfig",
> -                  "browser/config/mozconfigs/macosx-universal/l10n-mozconfig",
> +                  "browser/config/mozconfigs/macosx64/l10n-mozconfig",

This file is in the list twice.
Attachment #8815878 - Flags: review?(coop) → review+
(Assignee)

Comment 28

9 months ago
mozreview-review-reply
Comment on attachment 8815878 [details]
Bug 1295375 - Drop universal Mac builds;

https://reviewboard.mozilla.org/r/96662/#review97736

> This file is in the list twice.

Good catch - fixed.
(Assignee)

Comment 29

9 months ago
I also fixed the mozconfig whitelist, since the compare-mozconfigs check now fail properly from bug 1320761.
Comment hidden (mozreview-request)
(Assignee)

Comment 31

9 months ago
Created attachment 8817214 [details] [diff] [review]
fix-tests

:jgraham, this seems to be the minimum needed to fix these tests for OSX 64-bit only builds (see also #c25, #c26). Is this the right way to fix them? It's not obvious to me why there are several cases spelled out that appear like they could be collapsed, such as an 'if e10s' and an 'if not e10s' line that have the same result. Are the lines auto-generated from somewhere?

Also by grepping through *.ini, I can see a number of other tests that still reference '(processor == "x86") and (bits == "32")', but I don't know why they don't currently fail for non-universal builds. Do they all need to be updated as well?

I think the reason these changes are required is that the opt builds will now be x86_64 and 64-bits similar to the debug builds, whereas the universal opt builds seem to identify themselves as x86 and 32-bits.
Attachment #8817214 - Flags: feedback?(james)
Comment on attachment 8817214 [details] [diff] [review]
fix-tests

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

This looks like a reasonable change if the opt builds are now identifying as x86_64 rather than x86, but like you I'm pretty surprised that there aren't more cases that need to be updated.

The reason the metadata format is so awful is indeed that the files are autogenerated when we update the web-platform-tests from upstream, and the script that generates them doesn't know the full matrix of possible values (which in any case changes relatively often as we change platform versions and so on), so it doesn't do much optimisation of the output.
Attachment #8817214 - Flags: feedback?(james) → feedback+
(Assignee)

Comment 33

9 months ago
(In reply to James Graham [:jgraham] from comment #32)
> The reason the metadata format is so awful is indeed that the files are
> autogenerated when we update the web-platform-tests from upstream, and the
> script that generates them doesn't know the full matrix of possible values
> (which in any case changes relatively often as we change platform versions
> and so on), so it doesn't do much optimisation of the output.

Which script is this? Should I be updating that instead and re-running it instead of making these changes manually?
For such a small number of changes, making the alterations manually is fine. If you want to run the script you need to download the raw logs from treeherder e.g. using testing/web-platform/update/fetchlogs.py (with arguments <repo>, <short sha1>) and then run

mach wpt-update path/to/logs/*.log

Which will slowly churn through the logs and rewrite the metadata files. But this process has a few rough edges and limited benefits in this case.

Updated

9 months ago
Blocks: 1322402
Comment hidden (mozreview-request)
(Assignee)

Comment 36

9 months ago
Alrighty, I've just made these manual changes to get the tests green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=052319a84a5625db4af4acf6e17ae5d813ee2be2

Comment 37

9 months ago
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f82cd0199ab7
Drop universal Mac builds; r=coop,gps
Yay this landed! Boo, we didn't do bug 1310849 yet. I've locked updates to Firefox-mozilla-central-nightly-20161208030206 for the nightly channel, where the OS Version contains Darwin and buildID is < 20161208075029 (see rule id 481). ie all Mac updates won't go past 20161208075029.

After testing we can change this to just a Build Target of Darwin_x86-gcc3-u-i386-x86_64, which is what 32bit queries with. The fisrt build after this patch landed is using Darwin_x86_64-gcc3.
(Assignee)

Comment 39

9 months ago
(In reply to Nick Thomas [:nthomas] from comment #38)
> Yay this landed! Boo, we didn't do bug 1310849 yet. I've locked updates to
> Firefox-mozilla-central-nightly-20161208030206 for the nightly channel,
> where the OS Version contains Darwin and buildID is < 20161208075029 (see
> rule id 481). ie all Mac updates won't go past 20161208075029.

Doh, somehow I got into my head that we were going to sort out update issues for aurora and beyond. Thanks for sorting this out!

Comment 40

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f82cd0199ab7
Status: NEW → RESOLVED
Last Resolved: 9 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Ben/Jordan, could you have a look at the Balrog rules for Firefox on the nightly channel, to see why https://aus5.mozilla.org/update/6/Firefox/53.0a1/20161208075029/Darwin_x86_64-gcc3-u-i386-x86_64/en-US/nightly/Darwin%2015.6.0/NA/default/default/update.xml gets an update ?  Comment #38 is my attempt to block it with rule 461.

It’s not a problem right now, since the dropping-of-universal builds missed today’s m-c nightly, but it’ll be in the next one so I'd like to resolve it asap, and I'm have some technology issues right now.
Flags: needinfo?(bhearsum)
(In reply to Nick Thomas [:nthomas] from comment #41)
> Ben/Jordan, could you have a look at the Balrog rules for Firefox on the
> nightly channel, to see why
> https://aus5.mozilla.org/update/6/Firefox/53.0a1/20161208075029/
> Darwin_x86_64-gcc3-u-i386-x86_64/en-US/nightly/Darwin%2015.6.0/NA/default/
> default/update.xml gets an update ?  Comment #38 is my attempt to block it
> with rule 461.
> 
> It’s not a problem right now, since the dropping-of-universal builds missed
> today’s m-c nightly, but it’ll be in the next one so I'd like to resolve it
> asap, and I'm have some technology issues right now.

I think this is because the rule sets version to '<20161208075029', and the request you've pasted has that exact buildid. Presumably this should be '<=20161208075029', which (in combination with the OS Version field) catches all mac users on that build or older? Hoping to double check this with you before making the change, because I feel like I don't have full context.
Flags: needinfo?(bhearsum) → needinfo?(nthomas)
Thanks for looking Ben. I realized I needed to add a No-Update rule once mac users get to 20161208075029 - with that all mac updates are blocked.
Flags: needinfo?(nthomas)
Flags: needinfo?(jlund)
mshal and I finished the testing in bug 1310849 and we've re-enabled updates for 64bit builds. 32bit users stop at a buildID of 20161208075029. There's one more nightly after that, but it's on the same revision.

Comment 45

9 months ago
This probably is the final step to close Bug 787510?
(Reporter)

Updated

9 months ago
Blocks: 787510
and some small (2%) talos improvements in ts_paint (browser startup):
https://treeherder.mozilla.org/perf.html#/alerts?id=4478
(Reporter)

Updated

8 months ago
Blocks: 1278812

Updated

8 months ago
See Also: → bug 1325582

Updated

8 months ago
See Also: → bug 1325583

Updated

8 months ago
See Also: → bug 1325584
See bug 1325582, bug 1325583, and bug 1325584 for the update server work as this change merges upwards.
aurora is done now: https://bugzilla.mozilla.org/show_bug.cgi?id=1325582#c1
Depends on: 1333443

Comment 49

7 months ago
Added to Fx53 Aurora release notes.
relnote-firefox: --- → 53+
(Assignee)

Updated

6 months ago
See Also: → bug 1339182

Updated

6 months ago
Blocks: 1339182
Duplicate of this bug: 1231164
You need to log in before you can comment on or make changes to this bug.