Last Comment Bug 1295375 - Drop support for universal Mac builds in Firefox 53
: Drop support for universal Mac builds in Firefox 53
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: Unspecified Unspecified
-- normal (vote)
: mozilla53
Assigned To: Michael Shal [:mshal]
:
: Gregory Szorc [:gps] (away until 2017-03-20)
Mentors:
Depends on: npapi-eol 1310849 1333443
Blocks: 787510 1278812 1322402 1339182
  Show dependency treegraph
 
Reported: 2016-08-15 17:05 PDT by Gregory Szorc [:gps] (away until 2017-03-20)
Modified: 2017-02-14 20:53 PST (History)
23 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
53+

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
Bug 1295375 - Drop universal Mac builds; (58 bytes, text/x-review-board-request)
2016-11-30 14:11 PST, Michael Shal [:mshal]
coop: review+
gps: review+
Details | Review
fix-tests (5.20 KB, patch)
2016-12-07 12:12 PST, Michael Shal [:mshal]
james: feedback+
Details | Diff | Splinter Review

Description User image Gregory Szorc [:gps] (away until 2017-03-20) 2016-08-15 17:05:40 PDT
Per bug 1183613 comment 16, we can drop universal mac builds in Firefox 53.

mozilla-central becomes Firefox 53 on November 7.
Comment 1 User image Mike Hommey [:glandium] 2016-08-15 17:16:23 PDT
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.
Comment 2 User image Mike Hommey [:glandium] 2016-08-15 17:17:44 PDT
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.
Comment 3 User image Gregory Szorc [:gps] (away until 2017-03-20) 2016-08-15 18:36:03 PDT
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.
Comment 4 User image Ted Mielczarek [:ted.mielczarek] 2016-10-04 02:40:13 PDT
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).
Comment 5 User image Gregory Szorc [:gps] (away until 2017-03-20) 2016-11-22 15:28:02 PST
bsmedberg: can we proceed with this?
Comment 6 User image Daniel Glazman (:glazou) 2016-11-23 00:17:09 PST
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.
Comment 7 User image Benjamin Smedberg [:bsmedberg] 2016-11-28 13:20:10 PST
Yes gogogo!
Comment 8 User image Gregory Szorc [:gps] (away until 2017-03-20) 2016-11-28 13:56:43 PST
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)?
Comment 9 User image Gregory Szorc [:gps] (away until 2017-03-20) 2016-11-28 14:05:45 PST
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?
Comment 10 User image Rail Aliiev [:rail] ⌚️ET 2016-11-28 20:09:44 PST
302 jlund|releaseduty
Comment 11 User image Benjamin Smedberg [:bsmedberg] 2016-11-29 07:00:38 PST
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.
Comment 12 User image Mike Hommey [:glandium] 2016-11-29 23:38:55 PST
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.
Comment 13 User image Ted Mielczarek [:ted.mielczarek] 2016-11-30 02:51:53 PST
(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".
Comment 14 User image Chris Cooper [:coop] 2016-11-30 06:52:11 PST
(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.
Comment 15 User image Chris Cooper [:coop] 2016-11-30 06:53:26 PST
(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.
Comment 16 User image Chris Cooper [:coop] 2016-11-30 06:54:18 PST
(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.
Comment 17 User image Michael Shal [:mshal] 2016-11-30 11:00:59 PST
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).
Comment 18 User image Ted Mielczarek [:ted.mielczarek] 2016-11-30 11:04:16 PST
(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 19 User image Michael Shal [:mshal] 2016-11-30 14:11:43 PST Comment hidden (mozreview-request)
Comment 20 User image Gregory Szorc [:gps] (away until 2017-03-20) 2016-11-30 14:42:40 PST
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.
Comment 21 User image Gregory Szorc [:gps] (away until 2017-03-20) 2016-11-30 15:08:09 PST
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?
Comment 22 User image Michael Shal [:mshal] 2016-12-01 13:11:24 PST
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 23 User image Michael Shal [:mshal] 2016-12-01 13:21:23 PST Comment hidden (mozreview-request)
Comment 24 User image Gregory Szorc [:gps] (away until 2017-03-20) 2016-12-02 10:03:29 PST
Comment on attachment 8815878 [details]
Bug 1295375 - Drop universal Mac builds;

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

Good riddance.
Comment 25 User image Michael Shal [:mshal] 2016-12-02 12:08:34 PST
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
Comment 26 User image Benjamin Smedberg [:bsmedberg] 2016-12-02 12:26:44 PST
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
Comment 27 User image Chris Cooper [:coop] 2016-12-06 09:28:09 PST
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.
Comment 28 User image Michael Shal [:mshal] 2016-12-07 11:58:01 PST
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.
Comment 29 User image Michael Shal [:mshal] 2016-12-07 11:58:58 PST
I also fixed the mozconfig whitelist, since the compare-mozconfigs check now fail properly from bug 1320761.
Comment 30 User image Michael Shal [:mshal] 2016-12-07 12:01:26 PST Comment hidden (mozreview-request)
Comment 31 User image Michael Shal [:mshal] 2016-12-07 12:12:53 PST
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.
Comment 32 User image James Graham [:jgraham] 2016-12-07 12:24:35 PST
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.
Comment 33 User image Michael Shal [:mshal] 2016-12-07 12:31:13 PST
(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?
Comment 34 User image James Graham [:jgraham] 2016-12-07 12:39:45 PST
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.
Comment 35 User image Michael Shal [:mshal] 2016-12-07 17:41:14 PST Comment hidden (mozreview-request)
Comment 36 User image Michael Shal [:mshal] 2016-12-07 18:06:18 PST
Alrighty, I've just made these manual changes to get the tests green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=052319a84a5625db4af4acf6e17ae5d813ee2be2
Comment 37 User image Pulsebot 2016-12-08 10:36:47 PST
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f82cd0199ab7
Drop universal Mac builds; r=coop,gps
Comment 38 User image Nick Thomas [:nthomas] 2016-12-08 13:53:14 PST
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.
Comment 39 User image Michael Shal [:mshal] 2016-12-08 14:44:47 PST
(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 User image Carsten Book [:Tomcat] 2016-12-09 04:34:29 PST
https://hg.mozilla.org/mozilla-central/rev/f82cd0199ab7
Comment 41 User image Nick Thomas [:nthomas] 2016-12-09 10:18:38 PST
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.
Comment 42 User image Ben Hearsum (:bhearsum) 2016-12-09 13:58:12 PST
(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.
Comment 43 User image Nick Thomas [:nthomas] 2016-12-09 19:41:54 PST
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.
Comment 44 User image Nick Thomas [:nthomas] 2016-12-09 19:49:19 PST
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 User image Morpheus3k 2016-12-10 06:09:29 PST
This probably is the final step to close Bug 787510?
Comment 46 User image Joel Maher ( :jmaher) 2016-12-12 09:48:46 PST
and some small (2%) talos improvements in ts_paint (browser startup):
https://treeherder.mozilla.org/perf.html#/alerts?id=4478
Comment 47 User image Nick Thomas [:nthomas] 2016-12-23 02:44:07 PST
See bug 1325582, bug 1325583, and bug 1325584 for the update server work as this change merges upwards.
Comment 48 User image Jordan Lund (:jlund) 2017-01-23 11:15:13 PST
aurora is done now: https://bugzilla.mozilla.org/show_bug.cgi?id=1325582#c1
Comment 49 User image Ritu Kothari (:ritu) 2017-01-28 15:31:09 PST
Added to Fx53 Aurora release notes.

Note You need to log in before you can comment on or make changes to this bug.