1205881 - error: use of undeclared identifier 'NSVisualEffectMaterialMenu' with OS X 10.11 SDK

Status: RESOLVED INVALID

(Core :: Widget: Cocoa, defect)

Description

[(no longer active)]

I updated to XCode 7 which helpfully got rid of all of my SDKs but the 10.11 SDK, and now when I build Firefox, I get:

 0:04.51 In file included from /Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/widget/cocoa/Unified_mm_widget_cocoa0.mm:56:
 0:04.51 /Users/ehsan/moz/src/widget/cocoa/VibrancyManager.mm:274:31: error: use of undeclared identifier 'NSVisualEffectMaterialMenu'
 0:04.51       [effectView setMaterial:NSVisualEffectMaterialMenu];
 0:04.51                               ^

Comment 1

[(no longer active)]

Attachment: Define NSVisualEffectMaterialMenu unconditionally

Comment 2

[Markus Stange [:mstange]]

That's weird, NSVisualEffectMaterialMenu is defined in /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/System/Library/Frameworks/AppKit.framework/Versions/C/Headers/NSVisualEffectView.h , not sure why that isn't picked up.

Comment 3

[(no longer active)]

I did try including that header directly, but it is definitely not picked up no matter what I tried.

Comment 4

[Markus Stange [:mstange]]

I just compiled my mozilla-central tree with a mozconfig that doesn't have any SDK specified in it, and it succeeded without problems.

Doing "mach build widget/cocoa/Unified_mm_widget_cocoa0.i" I see the following in the generated file:
> [...]
> # 1 "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSVisualEffectView.h" 1 3
> /*
>     NSVisualEffectView.h
>     Application Kit
>     Copyright (c) 2014-2015, Apple Inc.
>     All rights reserved.
> */
> 
> 
> 
> 
> /* The main material that this view displays.
>  */
> 
> 
> typedef enum NSVisualEffectMaterial : NSInteger NSVisualEffectMaterial; enum NSVisualEffectMaterial : NSInteger {
>     // These first colors are abstract materials managed by AppKit and should be used when creating UI that needs to mimic these material types
>     // Many of these colors are dynamic and depend on the current NSAppearance set on the view (or its parent view)
>     NSVisualEffectMaterialAppearanceBased = 0, // Maps to Light or Dark, depending on the appearance set on the view
>     NSVisualEffectMaterialTitlebar = 3, // Mainly designed to be used for NSVisualEffectBlendingModeWithinWindow
>     NSVisualEffectMaterialMenu __attribute__((availability(macosx,introduced=10_11))) = 5,
> [...]

So it definitely seems to work on my side even without your fix.

My Xcode-beta is: "Version 7.0 beta 6 (7A192o)". Do you have a different version?

Comment 5

[(no longer active)]

I have Version 7.0 (7A220) which should be the latest stable XCode.  It looks like yours is old.

Note that when I upgraded XCode, it silently got rid of all of my SDKs, installed 10.11 SDK, and messed up the /usr/include symlink to make it point into a non-existing 10.10 SDK.  So there is clearly some XCode brokenness involved here.  That being said, I cannot build Firefox any more without this patch.  Is it really too bad to take it?

Comment 6

[Markus Stange [:mstange]]

For the record:

(In reply to Markus Stange [:mstange] from comment #4)
> My Xcode-beta is: "Version 7.0 beta 6 (7A192o)".

It turns out that I wasn't actually using this old outdated Xcode-beta; I was using regular the Xcode.app version 7.0.

On IRC Ehsan and I determined that his clang wasn't setting -isysroot to the 10.11 SDK and that his include search paths weren't containing the 10.11 SDK headers.

Comment 7

[(no longer active)]

FWIW I reinstalled Xcode and it didn't fix anything.

Comment 8

[(no longer active)]

Markus, what do you have in /Applications/Xcode.app/Contents/Developer/Platforms?  Is OSX 10.11 the only SDK that you have?  Adding --with-macos-sdk=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk to my mozconfig fixes the build for me.

I think the issue is that we don't attempt to auto-detect the OSX SDK, so if you happen to get unlucky and don't have one in a place that gets picked up by default by clang, we end up compiling without an OSX SDK!  I am amazed that this seems to be the only thing that breaks.

More interestingly, this gets fixed for me when I modify the build system to pick the default SDK in the same way that that xcrun does (according to what clang expects: <https://github.com/llvm-mirror/clang/blob/fc7ebbf5/lib/Driver/ToolChains.cpp#L431>)

Comment 9

[(no longer active)]

Attachment: Detect and use the default OS X SDK correctly

Comment 10

[(no longer active)]

Attachment: Detect and use the default OS X SDK correctly

Comment 11

[(no longer active)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1cdac11f616

Comment 12

[Markus Stange [:mstange]]

(In reply to Ehsan Akhgari (don't ask for review please) from comment #8)
> Markus, what do you have in
> /Applications/Xcode.app/Contents/Developer/Platforms?

MacOSX.platform
WatchOS.platform
WatchSimulator.platform
iPhoneOS.platform
iPhoneSimulator.platform

> Is OSX 10.11 the only SDK that you have?

Yes, that's the only SDK I have in /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs .

I have copies of older SDKs in ~/SDKs, but those only get used when I specify them manually.

>  Adding
> --with-macos-sdk=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.
> platform/Developer/SDKs/MacOSX10.11.sdk to my mozconfig fixes the build for
> me.

Ok.

> I think the issue is that we don't attempt to auto-detect the OSX SDK, so if
> you happen to get unlucky and don't have one in a place that gets picked up
> by default by clang, we end up compiling without an OSX SDK!

Do we know under which conditions clang picks up an SDK by default? Is it Apple clang vs regular clang?

> I am amazed that this seems to be the only thing that breaks.

I suspect that a lot more would break if your /System/Library/Frameworks/AppKit.framework/ directory didn't even have any Headers directory, as is the case on my machine...

> More interestingly, this gets fixed for me when I modify the build system to
> pick the default SDK in the same way that that xcrun does (according to what
> clang expects:
> <https://github.com/llvm-mirror/clang/blob/fc7ebbf5/lib/Driver/ToolChains.
> cpp#L431>)

It looks like you've done a lot of investigation into this. I don't really know what the code you linked to does, but using xcrun to determine the SDK path sounds reasonable.

Comment 13

[Markus Stange [:mstange]]

Comment on attachment 8666385 [details] [diff] [review]
Detect and use the default OS X SDK correctly

This looks good, but it needs to be reviewed by a build peer.
Do we know whether all clang version we support understand the SDKROOT env variable?

As a note, if this is the first command in the build that invokes a command line tool that's provided by Xcode, then this is the place where we'll fail when the user has just updated their Xcode version and not accepted the license agreement. Not sure if that matters in any way.

Comment 14

[(no longer active)]

(In reply to Markus Stange [:mstange] from comment #13)
> This looks good, but it needs to be reviewed by a build peer.

OK.

> Do we know whether all clang version we support understand the SDKROOT env
> variable?

Yes, this has been added in clang 3.2.

> As a note, if this is the first command in the build that invokes a command
> line tool that's provided by Xcode, then this is the place where we'll fail
> when the user has just updated their Xcode version and not accepted the
> license agreement. Not sure if that matters in any way.

We currently don't handle that scenario in any special way and the first time that the build runs clang it fails in mysterious ways (I hit that when I updated Xcode last week!)

Comment 15

[Steven Michaud [:smichaud] (Retired)]

Ehsan, what does 'xcode-select -p' return on your system?

We already have code in our build system that looks for XCode (using xcode-select), to find the SDKs directory.

Comment 16

[Steven Michaud [:smichaud] (Retired)]

https://hg.mozilla.org/mozilla-central/annotate/031db40e2b55/media/webrtc/trunk/build/mac/find_sdk.py

Comment 17

[(no longer active)]

(In reply to Steven Michaud [:smichaud] from comment #15)
> Ehsan, what does 'xcode-select -p' return on your system?

It returns the correct thing (/Applications/Xcode.app/Contents/Developer) but that is not interesting for this bug since the issue is not about side-by-side Xcode installations (as I only have one) but about clang not picking up the SDK.

> We already have code in our build system that looks for XCode (using
> xcode-select), to find the SDKs directory.

Wow that code is all wrong!  It literally guesses where the SDK is.  :-)

It seems like as a follow-up, we need to change that code to also use xcrun --sdk macosx --show-sdk-path.  But one step at a time.

Comment 18

[Steven Michaud [:smichaud] (Retired)]

Someone (besides me) needs to test Ehsan's patch on all supported versions of OS X (10.6.8 and up).  If this feature of xcrun (or even xcrun itself) is only available on some of them, we'll need to make sure we use it only on that major version and up.

Comment 19

[Steven Michaud [:smichaud] (Retired)]

There's an xcrun on OS X 10.6.8 and 10.7.5.  But I'd bet it only supports '--sdk macosx --show-sdk-path' on OS X 10.8.5 and up.  Or rather on the latest version of XCode available for that version of OS X.

Comment 20

[(no longer active)]

It works on 10.6 since that is what we use on our build machines, and also 10.10.  I have not tested this on every single version out there, obviously.

Comment 21

[Gregory Szorc [:gps]]

Comment on attachment 8666385 [details] [diff] [review]
Detect and use the default OS X SDK correctly

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

The logic change looks sound. But I'm not granting r+ yet because I /think/ this isn't a complete solution.

AC_DEFINE and AC_SUBST don't act on SDKROOT. This means the "export SDKROOT" is only valid for the length of the configure process tree. I'm pretty sure the bits that care about this variable are GYP processing, which happens as part of moz.build processing when running config.status. configure invokes config.status, so SDKROOT is being inherited there.

But I bet if you run `mach build-backend --diff` with this patch applied, you'll find that things revert because SDKROOT is not set in that process (only in configure).

I think you'll need to AC_SUBST the resolved path and then modify the GYP reading code, probably at https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/frontend/gyp_reader.py?offset=600#76 to read that variable and define it as SDKROOT for GYP. (Please don't globally define SDKROOT because it is a generic variable and the symbol appears elsewhere in the tree.)

Assuming I'm correct, you might want to ping glandium about the GYP foo, as he understands it better than me. You may also want to hit him up for next review if you touch GYP foo.

Comment 22

[(no longer active)]

(In reply to Gregory Szorc [:gps] from comment #21)
> AC_DEFINE and AC_SUBST don't act on SDKROOT. This means the "export SDKROOT"
> is only valid for the length of the configure process tree. I'm pretty sure
> the bits that care about this variable are GYP processing, which happens as
> part of moz.build processing when running config.status. configure invokes
> config.status, so SDKROOT is being inherited there.

Is that true?  The thing that needs to pick this variable up is not GYP processing, it's the clang invocations, and somehow this seems to fix the build bug for me...

> But I bet if you run `mach build-backend --diff` with this patch applied,
> you'll find that things revert because SDKROOT is not set in that process
> (only in configure).
> 
> I think you'll need to AC_SUBST the resolved path and then modify the GYP
> reading code, probably at
> https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/
> frontend/gyp_reader.py?offset=600#76 to read that variable and define it as
> SDKROOT for GYP. (Please don't globally define SDKROOT because it is a
> generic variable and the symbol appears elsewhere in the tree.)
> 
> Assuming I'm correct, you might want to ping glandium about the GYP foo, as
> he understands it better than me. You may also want to hit him up for next
> review if you touch GYP foo.

Can you please explain how GYP has anything to do with this bug?  I am really puzzled as to why we're talking about GYP processing here.

Comment 23

[Steven Michaud [:smichaud] (Retired)]

> It works on 10.6 since that is what we use on our build machines

Our 10.6 machines are hybrid monsters, with all sorts of things you'd normally only find on later versions of OS X.

Comment 24

[Gregory Szorc [:gps]]

Because when I search for SDKROOT in mozilla-central I see a bunch of referenced in GYP and no references in our build system.

How exactly is SDKROOT surviving being exported to an environment variable in configure to Clang invocations in a separate process tree? No matter what the answer, it needs to be documented with an inline comment because the answer isn't obvious.

Comment 25

[(no longer active)]

(In reply to Gregory Szorc [:gps] from comment #24)
> Because when I search for SDKROOT in mozilla-central I see a bunch of
> referenced in GYP and no references in our build system.

Comment 8 explains that it is clang that uses SDKROOT, not anything in our build system.

> How exactly is SDKROOT surviving being exported to an environment variable
> in configure to Clang invocations in a separate process tree? No matter what
> the answer, it needs to be documented with an inline comment because the
> answer isn't obvious.

Here is some bad news.  I cannot reproduce the original bug any more.  :(  Now I can build a pristine copy of mozilla-central.  Given that I have wasted way too much time on this already and I can't really do things such as test on all OSX versions, I'm gonna declare defeat here and let the next person to hit this bug figure the rest out.