Bug 152720

Summary: [GTK][OSX] JavaScriptCore fails to build on older versions of OS X due to CF_AVAILABLE usage
Product: WebKit Reporter: Jeremy Huddleston Sequoia <jeremyhu>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ggaren, keith_miller, mark.lam, msaboff, philip.chimento, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Mac   
OS: OS X 10.11   
See Also: https://bugs.webkit.org/show_bug.cgi?id=126433
https://bugs.webkit.org/show_bug.cgi?id=157574
Bug Depends on:    
Bug Blocks: 126492    
Attachments:
Description Flags
patch
none
Updated patch with ChangeLog entry none

Jeremy Huddleston Sequoia
Reported 2016-01-04 17:31:45 PST
Building webkit-gtk 2.11.2 on the MacPorts Mavericks buildbot failed due CF_AVAILABLE macro usage. These macros should only be used by the OS X system frameworks and not local builds. /opt/local/var/macports/build/_opt_mports_dports_www_webkit2-gtk/webkit2-gtk/work/webkitgtk-2.11.2/Source/JavaScriptCore/API/JSContextRef.h:144:71: error: expected '=' after '__NSi_10_10' JS_EXPORT JSStringRef JSGlobalContextCopyName(JSGlobalContextRef ctx) CF_AVAILABLE(10_10, 8_0); ^ /System/Library/Frameworks/CoreFoundation.framework/Headers/CFAvailability.h:95:81: note: expanded from macro 'CF_AVAILABLE' #define CF_AVAILABLE(_mac, _ios) __attribute__((availability(macosx,__NSi_##_mac))) ^ --- Elsewhere, I see these warnings, which seem to attempt to do that: /opt/local/var/macports/build/_opt_mports_dports_www_webkit2-gtk/webkit2-gtk/work/webkitgtk-2.11.2/Source/JavaScriptCore/API/WebKitAvailability.h:68:9: warning: 'CF_AVAILABLE' macro redefined #define CF_AVAILABLE(_mac, _ios) ^ /System/Library/Frameworks/CoreFoundation.framework/Headers/CFAvailability.h:95:9: note: previous definition is here #define CF_AVAILABLE(_mac, _ios) __attribute__((availability(macosx,__NSi_##_mac))) ^ --- Thus, it looks like WebKitAvailability.h needs to be included in JSContextRef.h before that CF_AVAILABLE usage. Also, please add an #undef before the #define in WebKitAvailability.h to silence that warning.
Attachments
patch (1.12 KB, patch)
2016-01-15 08:43 PST, Jeremy Huddleston Sequoia 		no flags
DetailsFormatted DiffDiff
Updated patch with ChangeLog entry (2.38 KB, patch)
2016-01-15 23:21 PST, Jeremy Huddleston Sequoia 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Jeremy Huddleston Sequoia
Comment 1 2016-01-14 15:10:27 PST
Actually, the issue seems to be that WebKitAvailability.h is guarded by: #if defined(__APPLE__) && !defined(BUILDING_GTK__) And thus has no effect when building webkit-gtk for some reason. Why was that done?
Jeremy Huddleston Sequoia
Comment 2 2016-01-14 15:23:35 PST
This regression was caused by: commit e47eb45c6bcb88b853858eb0a3aaecbbdcc3aa38 Author: mcatanzaro@igalia.com <mcatanzaro@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc> Date: Mon Oct 12 17:59:45 2015 +0000 webkit-gtk 2.3.3 fails to build on OS X - Conflicting type "Fixed" https://bugs.webkit.org/show_bug.cgi?id=126433 Patch by Philip Chimento <philip.chimento@gmail.com> on 2015-10-12 Reviewed by Philippe Normand Don't include CoreFoundation.h when building the GTK port. * Source/JavaScriptCore/API/WebKitAvailability.h: Add !defined(BUILDING_GTK__) to defined(__APPLE__). git-svn-id: http://svn.webkit.org/repository/webkit/trunk@190862 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Jeremy Huddleston Sequoia
Comment 3 2016-01-14 15:29:37 PST
Odd, that change looks fine to my eyes, and we should be getting: #define CF_AVAILABLE(_mac, _ios) out of WebKitAvailability.h. Something isn't adding up. I need to take a closer look at this.
Philip Chimento
Comment 4 2016-01-14 19:48:30 PST
The idea was that we wouldn't include CoreFoundation.h even if it was available, when building WebKitGTK, because some types included from CF conflicted with other types in WebKitGTK. I don't quite understand what's going wrong with that change.
Jeremy Huddleston Sequoia
Comment 5 2016-01-14 20:52:41 PST
Based on the other build warnings (redefinition of the macro), CF is getting included elsewhere as well. In some cases, the ordering works out, but in this case, it doesn't. I think a better approach might be to include it and then redefine the macro. Gonna give that a test tomorrow.
Jeremy Huddleston Sequoia
Comment 6 2016-01-15 08:43:53 PST
Created attachment 269059 [details] patch Yep, that did the trick. Patch attached.
Jeremy Huddleston Sequoia
Comment 7 2016-01-15 23:21:20 PST
Created attachment 269158 [details] Updated patch with ChangeLog entry
WebKit Commit Bot
Comment 8 2016-01-17 14:25:13 PST
Comment on attachment 269158 [details] Updated patch with ChangeLog entry Clearing flags on attachment: 269158 Committed r195184: <http://trac.webkit.org/changeset/195184>
WebKit Commit Bot
Comment 9 2016-01-17 14:25:17 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.