Closed
Bug 1295375
Opened 8 years ago
Closed 8 years ago
Drop support for universal Mac builds in Firefox 53
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(relnote-firefox 53+, firefox53 fixed)
RESOLVED
FIXED
mozilla53
People
(Reporter: gps, Assigned: mshal)
References
Details
Attachments
(2 files)
Per bug 1183613 comment 16, we can drop universal mac builds in Firefox 53.
mozilla-central becomes Firefox 53 on November 7.
Comment 1•8 years ago
|
||
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•8 years ago
|
||
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•8 years 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.
Comment 4•8 years ago
|
||
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).
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)
Reporter | ||
Comment 8•8 years 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•8 years 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)
Comment 11•8 years 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)
Comment 12•8 years ago
|
||
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•8 years ago
|
||
(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•8 years ago
|
||
(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•8 years ago
|
||
(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•8 years ago
|
||
(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•8 years 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
Comment 18•8 years ago
|
||
(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•8 years 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•8 years 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•8 years 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•8 years 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•8 years 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•8 years 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•8 years 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•8 years 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•8 years 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•8 years ago
|
||
: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 32•8 years ago
|
||
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•8 years 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?
Comment 34•8 years ago
|
||
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 hidden (mozreview-request) |
Assignee | ||
Comment 36•8 years 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•8 years ago
|
||
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f82cd0199ab7
Drop universal Mac builds; r=coop,gps
Comment 38•8 years ago
|
||
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•8 years 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•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment 41•8 years ago
|
||
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)
Comment 42•8 years ago
|
||
(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)
Comment 43•8 years ago
|
||
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)
Comment 44•8 years ago
|
||
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•8 years ago
|
||
This probably is the final step to close Bug 787510?
Comment 46•8 years ago
|
||
and some small (2%) talos improvements in ts_paint (browser startup):
https://treeherder.mozilla.org/perf.html#/alerts?id=4478
Updated•8 years ago
|
Updated•8 years ago
|
Updated•8 years ago
|
Comment 47•8 years ago
|
||
See bug 1325582, bug 1325583, and bug 1325584 for the update server work as this change merges upwards.
Comment 48•8 years ago
|
||
aurora is done now: https://bugzilla.mozilla.org/show_bug.cgi?id=1325582#c1
Added to Fx53 Aurora release notes.
relnote-firefox:
--- → 53+
Assignee | ||
Updated•8 years ago
|
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•