1642266 - Firefox no longer compiles with the macOS 10.14 SDK (error: reference to 'Rect' is ambiguous)

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Lina Butler [:lina]]

On latest m-c, a full clobber build of Firefox now fails with error: reference to 'Rect' is ambiguous for me. It looks like Dale ran into the same issue; here's his log.

Looking at my mozconfig, I noticed I had mine using the 10.14 SDK (which I had to do to work around bug 1487552...but looks like that's fixed now?), while Dale is using 10.13.

I asked in #fx-desktop-dev:mozilla.org on Matrix, and Emilio mentioned we use the 10.11 SDK on automation...so I'm giving that a try instead, using an SDK from here. But now I see bug 1349228 suggests removing --with-macos-sdk entirely.

So...what's the right SDK to build with? 😄 Now that bug 1487552 is fixed, should I remove --with-macos-sdk from my mozconfig, and stick with the latest Xcode and 10.15 SDK? Or should I use 10.11 to match automation?

Comment 1

[Lina Butler [:lina]]

Oh, for future reference, here's how I got the 10.11 SDK wired up:

1. Download the tarball from here, and untar it somewhere in my home folder. It sounds like Xcode updates will clobber old SDKs—though it probably doesn't matter in this case because 10.3 is never getting updated—so it's better to keep somewhere else.
2. cd /Applications/Xcode-10.3.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs
3. sudo ln -s /path/to/MacOSX10.11.sdk
4. plutil -remove MinimumSDKVersion /Applications/Xcode-10.3.app/Contents/Developer/Platforms/MacOSX.platform/Info.plist to remove the minimum version requirement.

It's building now, let's see if that fixes the error... 👍🏻 If not, I'll try the latest Xcode and 10.15 SDK.

Comment 2

[Mike Hommey [:glandium]]

Whatever code change caused this needs to be identified and fixed.

Comment 3

[:Gijs (out for now; he/him)]

I suspect https://hg.mozilla.org/mozilla-central/rev/9d7f77f76afa74947f1b015d4a097d71279d0850 , from bug 1218356, because the chain of includes from the log from comment #0 is:

79:24.26 In file included from /Users/dale/src/gecko/dom/ipc/ContentParent.cpp:234:
79:24.26 In file included from /Users/dale/src/gecko/media/webrtc/signaling/src/peerconnection/WebrtcGlobalParent.h:8:
79:24.26 In file included from /Users/dale/src/gecko/obj-x86_64-apple-darwin18.6.0/ipc/ipdl/_ipdlheaders/mozilla/dom/PWebrtcGlobalParent.h:18:
79:24.26 In file included from /Users/dale/src/gecko/obj-x86_64-apple-darwin18.6.0/dist/include/mozilla/media/webrtc/WebrtcGlobal.h:13:
79:24.26 In file included from /Users/dale/src/gecko/obj-x86_64-apple-darwin18.6.0/dist/include/mozilla/media/webrtc/WebrtcIPCTraits.h:14:
79:24.26 In file included from /Users/dale/src/gecko/obj-x86_64-apple-darwin18.6.0/dist/include/mtransport/transportlayerdtls.h:18:
79:24.26 In file included from /Users/dale/src/MacOSX-SDKs/MacOSX10.13.sdk/System/Library/Frameworks/Security.framework/Headers/CipherSuite.h:33:
79:24.26 In file included from /Users/dale/src/MacOSX-SDKs/MacOSX10.13.sdk/System/Library/Frameworks/CoreFoundation.framework/Headers/CFBase.h:77:

and this commit added the WebrtcIPCTraits.h header to WebrtcGlobal.h , and landed late on the 29th.

:ng, :dminor, can you check if that makes sense?

Off-hand, it'd seem to me that WebrtcIPCTraits.h does not need to include transportlayerdtls.h, which may be sufficient to fix this issue; or maybe better fixes are available, it's not like I'm familiar with this code...

Comment 4

[Dan Minor [:dminor]]

Nico's patch added serialization for DtlsDigest, so we need to include something for that. But it looks like including dtlsidentity.h may be enough and avoids pulling in the OS X system headers. I'm still waiting on the build on my ancient macbook. If it is good, I'll put it up for review. Otherwise, we'll have to wait for Nico.

Comment 5

[Dan Minor [:dminor]]

Attachment: Bug 1642266 - Include dtlsidentity.h in WebrtcIPCTraits.h; r=ng

Comment 7

[Robert Longson [:longsonr]]

That patch fixes it for me.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c33df12f568
Include dtlsidentity.h in WebrtcIPCTraits.h; r=ng

Comment 9

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Dan, thanks for looking at this.

Comment 10

[Nico Grunbaum [:ng, @chew:mozilla.org]]

I think a more permanent fix might be bug 1642449.

Comment 11

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/3c33df12f568

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:dminor, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 14

[Dan Minor [:dminor]]

I think we want the patch from Bug 1642449 instead.