I've only been focusing on the single-arch build in bug 921040, we'll need to sort out universal builds after that. I might need to fiddle with the toolchain (or build another one to target i386), and we'll have to check that the unify script has everything it needs etc.

Changing component and cc-ing some relevant people to get traction here. This blocks moving Mac builds to TaskCluster. I don't know whether this is something Wander could pick up on his own after his adventures with the Mac debug builds, but I suspect he'll need a build system resource to help. David should be able to help with scheduling there.

What is a Universal Build? obs: maybe this question confirms I need someone from build system to help working on that.

(In reply to Wander Lairson Costa [:wcosta] from comment #2) > What is a Universal Build? I believe in this case it means producing a .dmg that has both 32-bit and 64-bit builds inside, which is how we currently ship Firefox on Mac. After bug 1255588, we now only support OS X 10.9 and later. I believe these OS versions require 64-bit CPUs. Can we stop shipping universal builds and go to 64-bit only? Not sure who to ask, but maybe Ted knows.

We can't until we drop NPAPI.

Universal builds are where we do a 32-bit build and a 64-bit build and stitch them together into fat binaries. We've got a special mozconfig we use for that: https://dxr.mozilla.org/mozilla-central/source/build/macosx/universal/mozconfig.common I think the only fiddly bit here will be making sure that the cctools we're using can generate 32-bit binaries. If that works OK then it should just be a matter of mozconfig wrangling.

We had the discussion about dropping universal builds last year: https://lists.mozilla.org/pipermail/dev-platform/2015-August/011281.html The big blocker was having a replacement for Silverlight in Netflix, which should be Widevine.

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6) > We had the discussion about dropping universal builds last year: > https://lists.mozilla.org/pipermail/dev-platform/2015-August/011281.html > > The big blocker was having a replacement for Silverlight in Netflix, which > should be Widevine. AFAIK Widevine is available on all our platforms now. I'll talk to the Firefox product team and try to get traction for dropping universal builds.

(In reply to Mike Hommey [:glandium] from comment #4) > We can't until we drop NPAPI. Jim: cc-ing you here as the module owner for NPAPI. As glandium indicates, NPAPI is one of the few (possibly only) things preventing us from dropping universal builds on Mac. Is this true? If so, are there any plans to offer a 64-bit version and what would the timeline for that be?

It's not "NPAPI", it's "being OK with not supporting 32-bit plugins". Currently the most widely-used 32-bit only plugin is Silverlight.

I don't know what our current silverlight dependence is on mac, but I bet cpeterson does.

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6) > The big blocker was having a replacement for Silverlight in Netflix, which > should be Widevine. We support Widevine on Windows, Mac and (as of Beta 49, bug 1287925) Linux. Amazon Video uses Widevine on Mac and Netflix is currently beta testing it. So streaming sites do still depend on Silverlight DRM on Mac, but Widevine is available as an alternative. Anyways, we plan to drop support for Silverlight (32- and 64-bit, plus all NPAPI plugins other than Flash) in Firefox 52 (bug 1269807). I recommend waiting until 52 before dropping 32-bit Mac builds.

Firefox 52 goes to central on Sept 16, so if we could drop universal mac builds there and let it ride the release, that might be the best approach.

(In reply to Jonathan Griffin (:jgriffin) from comment #12) > Firefox 52 goes to central on Sept 16, so if we could drop universal mac > builds there and let it ride the release, that might be the best approach. I can get behind this plan. We'll still need some build system help to remove the universal packaging code, and to make sure 64-bit-only packaging works as expected. On the releng side, we'll need to make sure we have update rules in place to prevent updating 32-bit users. I suspect this population is low (or non-existent) given we stopped support for 10.6-10.8 in Firefox 49. (In reply to Chris Peterson [:cpeterson] from comment #11) > Anyways, we plan to drop support for Silverlight (32- and 64-bit, plus all > NPAPI plugins other than Flash) in Firefox 52 (bug 1269807). I recommend > waiting until 52 before dropping 32-bit Mac builds. This is perhaps implicit in the NPAPI EOL, but I'll clarify with the product team it's OK to stop providing universal builds with Firefox 52.

Mac debug builds are 64-bits only so of course 64-bits only packaging works as expected.

(In reply to Mike Hommey [:glandium] from comment #14) > Mac debug builds are 64-bits only so of course 64-bits only packaging works > as expected. Opt builds, especially in the context of nightlies, have signing requirements, yes?

(In reply to Chris Cooper [:coop] from comment #13) > This is perhaps implicit in the NPAPI EOL, but I'll clarify with the product > team it's OK to stop providing universal builds with Firefox 52. I spoke to bsmedberg about this. Firefox 52 is an ESR cycle and we're explicitly maintaining NPAPI support (spec. Silverlight) for the 52 ESR. For that reason, he'd like us to hold off on dropping universal binary support until Firefox 53, i.e. one cycle after the ESR branches. This has implications for platform support, the major one being that if we don't make the universal packaging work via cross-compilation, releng will need to preserve at least a small pool of Mac build machines to handle universal binaries on the 52 ESR branch. The existing Mac build machines are already old and I don't really want to support them for an additional 54-week ESR cycle that doesn't even start until 2017. Bumping the cycle until Firefox 53 does give us some more time to find resources for this project. I don't expect there will be much build system work needed to switch from universal->64bit-only on nightly. We will need a week or two of a build peer's time between now and the end of 2016 to help with cross-compiled universal builds.

I think the build peers may line up to drop universal builds support. Signing up to implement cross-compiled universal builds is another story completely ;)

I filed bug 1295375 to track dropping universal builds on Firefox 53.

(In reply to Chris Cooper [:coop] [away until Aug 29] from comment #15) > (In reply to Mike Hommey [:glandium] from comment #14) > > Mac debug builds are 64-bits only so of course 64-bits only packaging works > > as expected. > > Opt builds, especially in the context of nightlies, have signing > requirements, yes? It obviously would be good to run a test to ensure nothing goes awry here, but in general the universal build procedure takes a staged package for each architecture and slaps them together, with the resulting build having the same file layout as the two halves, but with fat binaries.

FYI, we have non-universal cross-mac opt builds enabled as tier-2 currently: https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/legacy/tasks/builds/opt_macosx64.yml example: https://tools.taskcluster.net/task-inspector/#cIf_J6xxTOWGSWXu_hjc3w/

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #19) > It obviously would be good to run a test to ensure nothing goes awry here, > but in general the universal build procedure takes a staged package for each > architecture and slaps them together, with the resulting build having the > same file layout as the two halves, but with fat binaries. If I wanted a non-Ted person to take a look at porting the universal packaging, where does that logic live?

I realized I actually don't know how this works now either, but it looks like we pass --with-unify-dist from the universal mozconfig: https://dxr.mozilla.org/mozilla-central/rev/1a5b53a831e5a6c20de1b081c774feb3ff76756c/build/macosx/universal/mozconfig.common#10 which triggers some special cases in packager.mk: https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk which drives some logic in packager.py: https://dxr.mozilla.org/mozilla-central/rev/1a5b53a831e5a6c20de1b081c774feb3ff76756c/toolkit/mozapps/installer/packager.py#344

Discussing with catlee today, mshal is going to spend two weeks investigating this. We expect this to go one of two ways: 1) It's easy, and after two weeks it's running in TC at tier 2. 2) It's harder than we thought, and Mike drops it to work on something else. The two week check-in is critical so this doesn't become a rabbit hole that Mike can't escape from. We can decide to continue, but it will be an explicit choice. An unsavory fallback option is to get the Mac TC worker running on the 10.7 builders and maintain a small pool of those for the duration of ESR 52.

(In reply to Chris Cooper [:coop] from comment #23) > Discussing with catlee today, mshal is going to spend two weeks > investigating this. Starting date TBD.

This looks promising so far. ted figured out that the toolchain we have in tooltool already supports 32-bit binary generation (even though the tools are x86_64-apple-darwin10-*), so we don't have to change those. The universal mozconfigs require some small tweaks (some of which I want to revisit before landing anything), and we have to add a configure check for lipo in order to use the cctools version on a cross build. I think we would just be landing this on ESR, so we can override the existing cross build with the universal one rather than creating a separate universal build type in TC.

WIP so far: https://treeherder.mozilla.org/#/jobs?repo=try&revision=578ecc3d32d8 $ file /Volumes/Nightly/Nightly.app/Contents/MacOS/firefox /Volumes/Nightly/Nightly.app/Contents/MacOS/firefox: Mach-O universal binary with 2 architectures /Volumes/Nightly/Nightly.app/Contents/MacOS/firefox (for architecture x86_64): Mach-O 64-bit executable x86_64 /Volumes/Nightly/Nightly.app/Contents/MacOS/firefox (for architecture i386): Mach-O executable i386

We might need to fix bug 1197325 if we're going to ship these to users.

In the last patch to get it working I had to remove --enable-dtrace and --enable-clang-plugin, as well as add --disable-startupcache. I managed to get --enable-clang-plugin working by adding MOZ_IMPLICIT to various constructors (thanks froydnj!), and --disable-startupcache is no longer needed by fixing packager.py. I still haven't figured out --enable-dtrace though - getting the package installed in the Linux builders should be straightforward, but even with dtrace available I get: In file included from /home/mshal/mozilla-central-git/js/src/builtin/RegExp.cpp:7: In file included from /home/mshal/mozilla-central-git/js/src/builtin/RegExp.h:10: In file included from /home/mshal/mozilla-central-git/js/src/vm/RegExpObject.h:13: In file included from /home/mshal/mozilla-central-git/js/src/jscntxt.h:18: /home/mshal/mozilla-central-git/js/src/vm/Runtime.h:73:8: error: thread-local storage is not supported for the current target extern __thread mozilla::detail::ThreadLocal<PerThreadData*> TlsPerThreadData; ^ In file included from /home/mshal/mozilla-central-git/js/src/builtin/RegExp.cpp:20: In file included from /home/mshal/mozilla-central-git/js/src/jsobjinlines.h:24: In file included from /home/mshal/mozilla-central-git/js/src/vm/Probes.h:11: /home/mshal/mozilla-central-git/obj-x86_64-apple-darwin/x86_64/js/src/javascript-trace.h:19:124: error: argument to 'section' attribute is not valid for this target: mach-o section specifier requires a segment and section separated by a comma __extension__ extern unsigned short javascript_function__entry_semaphore __attribute__ ((unused)) __attribute__ ((section (".probes"))); (long list of errors continues...) Do we need dtrace to call this a success or is it not really needed? To clarify my earlier comment about ESR, what I mean is I think we should probably pick whether we want the cross Taskcluster build to be single-arch or universal, rather than build both in a single branch. Historically we've called the universal build "macosx64-opt", and I don't think it makes sense to introduce a new name for something that's going away (or even more confusingly, calling the single-arch opt build something other than macosx64-opt). So we could either land this patch on central to temporarily have m-c do universal builds on Taskcluster, and back it out to go back to the single arch at the appropriate time, or we could just land it on the ESR branch and keep the cross m-c build as it is now.

froydnj to review *.cpp / *.h for the clang plugin changes, ted for the rest. Thanks!

(In reply to Michael Shal [:mshal] from comment #28) > Do we need dtrace to call this a success or is it not really needed? I'll let ted or froydnj comment as to how important dtrace is, or whether it needs to be in build that is destined for life on ESR-only (see below). > To clarify my earlier comment about ESR, what I mean is I think we should > probably pick whether we want the cross Taskcluster build to be single-arch > or universal, rather than build both in a single branch. Historically we've > called the universal build "macosx64-opt", and I don't think it makes sense > to introduce a new name for something that's going away (or even more > confusingly, calling the single-arch opt build something other than > macosx64-opt). So we could either land this patch on central to temporarily > have m-c do universal builds on Taskcluster, and back it out to go back to > the single arch at the appropriate time, or we could just land it on the ESR > branch and keep the cross m-c build as it is now. Universal builds on TC exist *solely* so that we don't need to preserve Mac build hardware for the lifecycle of Firefox 52 ESR. We've made a commitment to support NPAPI plugins that are only available in 32-bit mode (read: Silverlight) for Firefox 52 ESR, and being able to build these on Linux-in-AWS rather than Mac means we don't need to preserve aging (5+ years old now) Mac build hardware, or need to invest time/effort/money in getting the universal build working on new Mac hardware. As of Firefox 53, CI and nightly builds will switch to being 64bit-only once releng is able to do the update validation for moving people from universal builds to single-arch builds (bug 1310849). The plan is to get universal builds running at tier 2 on m-c ASAP, and then uplift to aurora if need be so we can make the Firefox 52 ESR train. I don't think we'll ever have universal builds running at tier 1 on m-c. Does this make sense?

I think it would be worth letting someone with clang familiarity take a look at that dtrace compile error just to see if it's something silly we're missing. Since we are planning on shipping these to users (even if only for ESR) it'd be best if we can make the build configuration match what we're currently shipping. If it is not a simple fix then dropping it should be fine.

Comment on attachment 8805571 [details] Bug 1183613 - Cross compile universal OSX builds in Taskcluster; https://reviewboard.mozilla.org/r/89338/#review88556 Cool, I'm glad to see that marking everything `MOZ_IMPLICIT` allowed you to make progress! Unfortunately, I think all of these should be marked `explicit` instead. :( Although I'm not a JS peer, I don't think we can go wrong by making things `explicit`. So, here's the compromise: 1. I'll r+ this patch with `s/MOZ_IMPLICIT/explicit/`. 2. You push to try or do whatever testing you need to do. 3. If that works, then great! We're done. 4. If that doesn't work, then ask :jandem what the right way forward is. ::: js/src/asmjs/WasmBaselineCompile.cpp:211 (Diff revision 1) > class ScratchI32 > { > # ifdef DEBUG > BaseCompiler& bc; > public: > ScratchI32(BaseCompiler& bc) : bc(bc) { This should be `explicit` to match the below. ::: js/src/asmjs/WasmBaselineCompile.cpp:221 (Diff revision 1) > MOZ_ASSERT(bc.scratchRegisterTaken()); > bc.setScratchRegisterTaken(false); > } > # else > public: > - ScratchI32(BaseCompiler& bc) {} > + MOZ_IMPLICIT ScratchI32(BaseCompiler& bc) {} This and all other `MOZ_IMPLICIT` changes should be `explicit` instead. (Partly adding this as an issue so MozReview won't let you autoland...)

(In reply to Chris Cooper [:coop] from comment #31) > (In reply to Michael Shal [:mshal] from comment #28) > > Do we need dtrace to call this a success or is it not really needed? > > I'll let ted or froydnj comment as to how important dtrace is, or whether it > needs to be in build that is destined for life on ESR-only (see below). My knee-jerk reaction is that it's not that important, but I don't know how useful it is to people. I know I've used dtrace on our OS X official builds, but I'm not sure if --disable-dtrace would make using dtrace impossible, less feasible, or wouldn't change a thing. I think we only have --enable-dtrace to enable bits of the JIT to be introspected, but I don't think we actually *use* that support... That being said, the __thread error in 28 is peculiar. OS X supports thread-local storage, but I think it didn't support it in Darwin 10 (which is 10.6, according to wikipedia), which is the version your patches compile against, and support only arrived in Darwin 11 (10.7). Spelunking through clang source appears to confirm that. Can you bump DARWIN_VERSION in the cross-compile case in build/macosx/universal/mozconfig.common to 11 and see if that fixes things? (Maybe try 12 if 11 doesn't work?) I'm a little surprised that __thread error didn't bite you in other places... As for the javascript-trace.h error, my guess is that the dtrace tool on Linux doesn't know how to generate dtrace headers and whatnot for the cross-compile case?

Huh, so yeah, there's that: cross-builds don't have a prefilled startup cache. Someone from product management will have to vouch for that, as that's going to be a perf regression on first run after install/upgrade. The interesting consequence is that if we accept to do it for mac, we might as well also accept to do it for windows and linux, which would make the installer smaller too.

:jandem, see #c28 and #c33 for some context. I was able to --enable-clang-plugin in a cross OSX universal build by adding 'explicit' to a few constructors, but AutoReleased in Instruments.cpp fails unless I use MOZ_IMPLICIT. Is that ok to use that there or is there some other fix? Here's the failed try push using 'explicit' for everything: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa4caf57be6b And here's a working one using 'explicit' for everything except AutoReleased, which uses 'MOZ_IMPLICIT': https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b8af6cfbb57

(In reply to Mike Hommey [:glandium] from comment #35) > Huh, so yeah, there's that: cross-builds don't have a prefilled startup > cache. Someone from product management will have to vouch for that, as > that's going to be a perf regression on first run after install/upgrade. The > interesting consequence is that if we accept to do it for mac, we might as > well also accept to do it for windows and linux, which would make the > installer smaller too. This is something that will also bite us as we update users from universal builds to cross-compiled 64-bit opt in Firefox 53 and later. I'll get a bug on file.

(In reply to Chris Cooper [:coop] from comment #37) > This is something that will also bite us as we update users from universal > builds to cross-compiled 64-bit opt in Firefox 53 and later. I'll get a bug > on file. Filed bug 1314154

(In reply to Michael Shal [:mshal] from comment #36) > but AutoReleased in Instruments.cpp fails unless I use > MOZ_IMPLICIT. Is that ok to use that there or is there some other fix? Yeah, rs=me to add MOZ_IMPLICIT there. It seems reasonable and this code isn't really used anyway.

It's looking like we will drop 32-bit support in 53. 52 will roll out (including to ESR) before TC builds for OS X are tier 1. We do not want to change platforms for point releases, so ESR52 will be on Buildbot until it sunsets. Given all of that, I don't think we will ever make universal builds on TaskCluster. In which case, maybe this is WONTFIX? But, cool that we identified the startup-cache issue.

(In reply to Dustin J. Mitchell [:dustin] from comment #40) > It's looking like we will drop 32-bit support in 53. 52 will roll out > (including to ESR) before TC builds for OS X are tier 1. We do not want to > change platforms for point releases, so ESR52 will be on Buildbot until it > sunsets. > > Given all of that, I don't think we will ever make universal builds on > TaskCluster. In which case, maybe this is WONTFIX? Our impetus is to get off of Mac hardware for builds, preferably well before a change in datacenters. There are the obvious scaling wins of being able to generate Mac builds in AWS. If we can't build universal binaries on Linux, we're committed to supporting the existing, 5-year-old Mac builders for the lifecycle of ESR 52 which is scheduled for EOL in 2018. If we have universal build working via cross-compilation, I'll push to have them backported to ESR if we don't make the cutoff.

OK, that makes sense. I'll consider it orthogonal to the TC migration, though, since if I read between the lines you'd backport universal cross-compiles to Buildbot, rather than porting the whole ESR to TaskCluster.

I think the issue of where we actually run the builds is not that big of a deal, honestly. :)

What do we have to do to get this running at tier 2 in TC alongside the existing opt and debug builds? We can close this bug out at that point.

The patch here will replace the existing opt cross-OSX build with a universal build. We don't do universal debug builds, so those will be unchanged (and existing buildbot builds are unchanged). I think we just need a build-peer signoff. ted, if you don't have time, can you redirect to someone else?

(In reply to Michael Shal [:mshal] from comment #45) > The patch here will replace the existing opt cross-OSX build with a > universal build. We don't do universal debug builds, so those will be > unchanged (and existing buildbot builds are unchanged). I think we just need > a build-peer signoff. We don't want to replace the opt build. Indeed, on non-ESR branches that single-arch config will be the new normal. I'm just looking for an in-tree universal config so we can uplift as necessary.

Comment on attachment 8805571 [details] Bug 1183613 - Cross compile universal OSX builds in Taskcluster; https://reviewboard.mozilla.org/r/89338/#review93496 This seems surprisingly painless. I'm impressed! ::: build/macosx/universal/mozconfig.common:8 (Diff revision 1) > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > mk_add_options MOZ_UNIFY_BDATE=1 > > +if test `uname -s` = Linux; then > + DARWIN_VERSION=10 I would just hardcode this all the time. I don't think it has any effect beyond being used as the toolchain prefix. ::: build/macosx/universal/mozconfig.common:48 (Diff revision 1) > > # These must be set for cross builds, and don't hurt straight builds. > - RANLIB=ranlib > - AR=ar > + RANLIB="${TOOLCHAIN_PREFIX}ranlib" > + AR="${TOOLCHAIN_PREFIX}ar" > AS=$CC > LD=ld Does this need to be changed or is the build system smart enough to pick up the right ld somehow?

(In reply to Michael Shal [:mshal] from comment #45) > The patch here will replace the existing opt cross-OSX build with a > universal build. We don't do universal debug builds, so those will be > unchanged (and existing buildbot builds are unchanged). I think we just need > a build-peer signoff. Oh yeah, so, the patch here looks OK, but like coop said we don't want to *replace* the existing cross-opt build, for two reasons: 1) As coop pointed out, we're going to drop universal mac builds soon and we want to ensure that single-arch opt builds continue to work without any surprises. 2) The opt builds are actually used by people doing artifact builds on OS X right now, specifically because they are *not* universal, so they're faster to download. Can you duplicate the taskcluster build config to make a separate universal build job?

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #48) > Can you duplicate the taskcluster build config to make a separate universal > build job? Yep, working on that now.

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #47) > > LD=ld > > Does this need to be changed or is the build system smart enough to pick up > the right ld somehow? The build system doesn't actually care about LD at all. I figured that out while working on bug 1317504 (and will morph the bug accordingly)

(In reply to Mike Hommey [:glandium] from comment #50) > The build system doesn't actually care about LD at all. I figured that out > while working on bug 1317504 (and will morph the bug accordingly) Meaning we'll want to start using ${TOOLCHAIN_PREFIX}ld now since LD will matter in the future? Or LD is just going to disappear and I should remove the assignment?

Don't touch it, I'll remove the irrelevant stuff in bug 1317504.

(In reply to Mike Hommey [:glandium] from comment #52) > Don't touch it, I'll remove the irrelevant stuff in bug 1317504. Perfect!

Ok, I created a separate cross-universal OSX build. It shows up as "Bu" under the tc[tier 2] group in OS X 10.7 opt: https://treeherder.mozilla.org/#/jobs?repo=try&revision=da0759e903740f4ab1b15031e7b8020b935dd043

Pushed by mshal@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f47ea54bdcf6 Cross compile universal OSX builds in Taskcluster; r=froydnj,ted

Thanks for the hard work here, Mike. Discussing with Kim today, I think there are two things left to do here: 1) Make cross-compiled universal builds a non-default option on Try; and 2) Uplift this code to the Firefox 52 train so it will make it onto the new ESR branch when that's cut. Am I missing anything here?

(In reply to Chris Cooper [:coop] from comment #57) > 1) Make cross-compiled universal builds a non-default option on Try; and > 2) Uplift this code to the Firefox 52 train so it will make it onto the new > ESR branch when that's cut. > > Am I missing anything here? I think we still need some direction in bug 1314154 for the startup cache issue. We are in a position now where we can remove universal build support in m-c, right? (bug 1295375) IMO we should uplift this, and then subsequently remove it from m-c as part killing universal build support. That way it will be a part of CI builds in 52-based branches, but not part of try builds for most people who push off of m-c. Does that sound reasonable? Or is there something else you want to address with part 1) that I'm missing?

As I said in comment 27, we probably need to also fix bug 1197325 if we're going to ship these to users. When I stood up the cross-builds initially the focus was on getting debug builds working. We noticed there were some quirks with the dmg that's generated, that bug should fix most of them but we haven't done any real concerted testing around running these builds. (In general we haven't been running automated tests *at all* on the cross-mac builds, which is pretty worrying.)

Comment on attachment 8805571 [details] Bug 1183613 - Cross compile universal OSX builds in Taskcluster; Approval Request Comment [Feature/regressing bug #]: 1183613 - this gives us flexibility to ship ESRs from Taskcluster instead of buildbot [User impact if declined]: Should be no user impact [Describe test coverage new/current, TreeHerder]: Currently in m-c, no new tests. [Risks and why]: Low risk, since this creates a new build and has minimal impact on existing builds. [String/UUID change made/needed]: None

Comment on attachment 8805571 [details] Bug 1183613 - Cross compile universal OSX builds in Taskcluster; this will help with esr52, take in aurora