Switch from using PLATFORM_NAME to SDK selectors in the WebCore, WebInspectorUI, WebKit and WebKit2 projects. See also: Bug 138813: FeatureDefines.xcconfig: Switch from using PLATFORM_NAME to SDK selectors Bug 139212: Switch from using PLATFORM_NAME to SDK selectors in ANGLE, bmalloc, gtest, JavaScriptCore, WTF
Created attachment 243081 [details] Patch v1 (Testing build; DO NOT REVIEW YET)
Comment on attachment 243081 [details] Patch v1 (Testing build; DO NOT REVIEW YET) View in context: https://bugs.webkit.org/attachment.cgi?id=243081&action=review > Source/WebCore/Configurations/WebCore.xcconfig:58 > +INSTALL_PATH_ios = $(SYSTEM_LIBRARY_DIR)/PrivateFrameworks; > +INSTALL_PATH[sdk=iphoneos*] = $(INSTALL_PATH_ios); > +INSTALL_PATH_ACTUAL[sdk=iphonesimulator*] = $(INSTALL_PATH_ios); > +INSTALL_PATH[sdk=macosx*] = $(WEBCORE_FRAMEWORKS_DIR); FYI, the INSTALL_PATH settings below in WebCoreTestSupport.xcconfig do NOT override the above settings (see next comment for the reason): INSTALL_PATH = $(INSTALL_PATH_$(CONFIGURATION)); INSTALL_PATH_Production[sdk=iphonesimulator*] = $(INDIGO_INSTALL_PATH_PREFIX)/usr/local/lib; INSTALL_PATH_Production = /usr/local/lib; > Source/WebCore/Configurations/WebCore.xcconfig:68 > +OTHER_LDFLAGS_BASE_ios = $(OTHER_LDFLAGS_BASE) -framework CFNetwork -framework CoreGraphics -framework CoreText -framework Foundation -framework GraphicsServices -framework ImageIO -framework OpenGLES -lMobileGestalt; > +OTHER_LDFLAGS[sdk=iphoneos*] = $(inherited) $(OTHER_LDFLAGS_BASE_ios) -framework IOSurface; > +OTHER_LDFLAGS[sdk=iphonesimulator*] = $(inherited) $(OTHER_LDFLAGS_BASE_ios); > +OTHER_LDFLAGS[sdk=macosx*] = $(inherited) $(OTHER_LDFLAGS_BASE) -sub_library libobjc -umbrella WebKit -allowable_client WebCoreTestSupport -allowable_client WebKit2 -allowable_client WebKitLegacy -framework ApplicationServices -framework AudioUnit -framework Carbon -framework Cocoa -framework IOSurface -framework OpenGL; This change causes this error when building DumpRenderTree: ld: cannot link directly with /Volumes/Data/Build/Release/libWebCoreTestSupport.dylib. Link against the umbrella framework 'WebKit.framework' instead. for architecture x86_64 Here's the reason: WebCoreTestSupport.xcconfig has OTHER_LDFLAGS defined thusly: OTHER_LDFLAGS = $(ASAN_OTHER_LDFLAGS); However, because the [sdk=] selectors are *more specific* than the variable without an [sdk=] selector, they take precedence. Pain.
Created attachment 243230 [details] Patch v2 (Testing build; DO NOT REVIEW unless is passes)
Created attachment 243242 [details] Patch v3
Comment on attachment 243242 [details] Patch v3 View in context: https://bugs.webkit.org/attachment.cgi?id=243242&action=review > Source/WebCore/Configurations/WebCoreTestSupport.xcconfig:38 > +INSTALL_PATH_ACTUAL[sdk=iphonesimulator*] = $(INSTALL_PATH_ACTUAL_$(CONFIGURATION)); > +INSTALL_PATH_ACTUAL_Production[sdk=iphonesimulator*] = $(INSTALL_PATH_$(CONFIGURATION)); This could be simplified to: INSTALL_PATH_ACTUAL[sdk=iphonesimulator*] = $(INSTALL_PATH_$(CONFIGURATION));
Comment on attachment 243242 [details] Patch v3 I think Mark Rowe should review. Looks great to me.
Created attachment 243295 [details] Patch v4
(In reply to comment #7) > Created attachment 243295 [details] > Patch v4 This patch makes the following changes over "Patch v3": - Simplifies INSTALL_PATH_ACTUAL in WebCoreTestSupport.xcconfig per Comment #5. - Sorts INSTALL_PATH_ACTUAL and INSTALL_PATH_ios in WebKitLegacy.xcconfig and WebKit2.xcconfig. - Fixes definition of OTHER_LDFLAGS in PluginProcessShim.xcconfig to use OTHER_LDFLAGS_PLATFORM pattern.
(In reply to comment #8) > (In reply to comment #7) > > Created attachment 243295 [details] > > Patch v4 > > This patch makes the following changes over "Patch v3": > > - Simplifies INSTALL_PATH_ACTUAL in WebCoreTestSupport.xcconfig per Comment > #5. > - Sorts INSTALL_PATH_ACTUAL and INSTALL_PATH_ios in WebKitLegacy.xcconfig > and WebKit2.xcconfig. > - Fixes definition of OTHER_LDFLAGS in PluginProcessShim.xcconfig to use > OTHER_LDFLAGS_PLATFORM pattern. I just installed the patchutils package and fixed the path to interdiff: https://bugs.webkit.org/attachment.cgi?oldid=243242&action=interdiff&newid=243295&headers=1
Comment on attachment 243295 [details] Patch v4 View in context: https://bugs.webkit.org/attachment.cgi?id=243295&action=review > Source/WebCore/Configurations/Base.xcconfig:109 > WEBCORE_SQLITE3_HEADER_SEARCH_PATHS = $(NEXT_ROOT)/usr/local/include/WebCoreSQLite3; > -SQLITE3_HEADER_SEARCH_PATHS = $(SQLITE3_HEADER_SEARCH_PATHS_$(PLATFORM_NAME)); > -SQLITE3_HEADER_SEARCH_PATHS_macosx = $(SQLITE3_HEADER_SEARCH_PATHS_macosx_$(TARGET_MAC_OS_X_VERSION_MAJOR)); > +SQLITE3_HEADER_SEARCH_PATHS = $(SQLITE3_HEADER_SEARCH_PATHS_$(PLATFORM_NAME)_$(TARGET_MAC_OS_X_VERSION_MAJOR)); I don't think either of these settings is used for anything at this point. > Source/WebCore/Configurations/Base.xcconfig:118 > +TOOLCHAINS = $(TOOLCHAINS_$(PLATFORM_NAME)_$(MAC_OS_X_VERSION_MAJOR)); Is there a reason to do the OS X case in this fashion rather than using TOOLCHAINS[sdk=macosx*] = $(TOOLCHAINS_macosx_$(MAC_OS_X_VERSION_MAJOR))? > Source/WebCore/Configurations/WebCore.xcconfig:36 > +EXPORTED_SYMBOLS_FILE_i386[sdk=iphonesimulator*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebCore/WebCore.LP64.i386.exp; > +EXPORTED_SYMBOLS_FILE_i386[sdk=macosx*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebCore/WebCore.exp; > +EXPORTED_SYMBOLS_FILE_x86_64[sdk=iphonesimulator*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebCore/WebCore.LP64.x86_64.exp; > +EXPORTED_SYMBOLS_FILE_x86_64[sdk=macosx*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebCore/WebCore.LP64.exp; I think we'll be able to update this to use [arch=foo] in the future rather than the current nested expansions. > Source/WebCore/Configurations/WebCore.xcconfig:43 > +FRAMEWORK_SEARCH_PATHS[sdk=iphone*] = $(FRAMEWORK_SEARCH_PATHS_ios_$(CONFIGURATION)); > +FRAMEWORK_SEARCH_PATHS_ios_Debug = $(BUILT_PRODUCTS_DIR) $(PRODUCTION_FRAMEWORKS_DIR); > +FRAMEWORK_SEARCH_PATHS_ios_Release = $(FRAMEWORK_SEARCH_PATHS_ios_Debug); > +FRAMEWORK_SEARCH_PATHS_ios_Production = $(PRODUCTION_FRAMEWORKS_DIR); > +FRAMEWORK_SEARCH_PATHS[sdk=macosx*] = $(STAGED_FRAMEWORKS_SEARCH_PATH) $(FRAMEWORK_SEARCH_PATHS); And this to use [config=Foo]. > Source/WebCore/Configurations/WebCore.xcconfig:62 > +DYLIB_INSTALL_NAME_BASE = $(DYLIB_INSTALL_NAME_BASE_PLATFORM); > +DYLIB_INSTALL_NAME_BASE_PLATFORM[sdk=iphoneos*] = $(DYLIB_INSTALL_NAME_BASE_ios); > +DYLIB_INSTALL_NAME_BASE_PLATFORM[sdk=iphonesimulator*] = $(SDKROOT)$(DYLIB_INSTALL_NAME_BASE_ios); > +DYLIB_INSTALL_NAME_BASE_PLATFORM[sdk=macosx*] = $(NORMAL_WEBCORE_FRAMEWORKS_DIR); Why is the indirection through DYLIB_INSTALL_NAME_BASE_PLATFORM necessary? Is it because DYLIB_INSTALL_NAME_BASE is set at the same level by WebCoreTestSupport.xcconfig after it includes this file? > Source/WebInspectorUI/Configurations/DebugRelease.xcconfig:7 > +MACOSX_DEPLOYMENT_TARGET = $(MACOSX_DEPLOYMENT_TARGET_$(PLATFORM_NAME)_$(TARGET_MAC_OS_X_VERSION_MAJOR)); Why is this not using [sdk=macosx*]? > Source/WebInspectorUI/Configurations/DebugRelease.xcconfig:23 > +SDKROOT = $(SDKROOT_$(PLATFORM_NAME)_$(USE_INTERNAL_SDK)); Ditto. > Source/WebInspectorUI/Configurations/Version.xcconfig:10 > +SYSTEM_VERSION_PREFIX = $(SYSTEM_VERSION_PREFIX_$(PLATFORM_NAME)_$(TARGET_MAC_OS_X_VERSION_MAJOR)); Ditto.
(In reply to comment #10) > Comment on attachment 243295 [details] > Patch v4 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=243295&action=review > > > Source/WebCore/Configurations/Base.xcconfig:109 > > WEBCORE_SQLITE3_HEADER_SEARCH_PATHS = $(NEXT_ROOT)/usr/local/include/WebCoreSQLite3; > > -SQLITE3_HEADER_SEARCH_PATHS = $(SQLITE3_HEADER_SEARCH_PATHS_$(PLATFORM_NAME)); > > -SQLITE3_HEADER_SEARCH_PATHS_macosx = $(SQLITE3_HEADER_SEARCH_PATHS_macosx_$(TARGET_MAC_OS_X_VERSION_MAJOR)); > > +SQLITE3_HEADER_SEARCH_PATHS = $(SQLITE3_HEADER_SEARCH_PATHS_$(PLATFORM_NAME)_$(TARGET_MAC_OS_X_VERSION_MAJOR)); > > I don't think either of these settings is used for anything at this point. Will be removed in Patch v5. > > Source/WebCore/Configurations/Base.xcconfig:118 > > +TOOLCHAINS = $(TOOLCHAINS_$(PLATFORM_NAME)_$(MAC_OS_X_VERSION_MAJOR)); > > Is there a reason to do the OS X case in this fashion rather than using > TOOLCHAINS[sdk=macosx*] = $(TOOLCHAINS_macosx_$(MAC_OS_X_VERSION_MAJOR))? The line above will trigger the Xcode 5.1.1 bug on Mac OS X 10.8. > > Source/WebCore/Configurations/WebCore.xcconfig:36 > > +EXPORTED_SYMBOLS_FILE_i386[sdk=iphonesimulator*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebCore/WebCore.LP64.i386.exp; > > +EXPORTED_SYMBOLS_FILE_i386[sdk=macosx*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebCore/WebCore.exp; > > +EXPORTED_SYMBOLS_FILE_x86_64[sdk=iphonesimulator*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebCore/WebCore.LP64.x86_64.exp; > > +EXPORTED_SYMBOLS_FILE_x86_64[sdk=macosx*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebCore/WebCore.LP64.exp; > > I think we'll be able to update this to use [arch=foo] in the future rather > than the current nested expansions. Agreed, but I don't want to change too much more with this patch. It's already pretty big, and I'd rather isolate that change to a separate commit. > > Source/WebCore/Configurations/WebCore.xcconfig:43 > > +FRAMEWORK_SEARCH_PATHS[sdk=iphone*] = $(FRAMEWORK_SEARCH_PATHS_ios_$(CONFIGURATION)); > > +FRAMEWORK_SEARCH_PATHS_ios_Debug = $(BUILT_PRODUCTS_DIR) $(PRODUCTION_FRAMEWORKS_DIR); > > +FRAMEWORK_SEARCH_PATHS_ios_Release = $(FRAMEWORK_SEARCH_PATHS_ios_Debug); > > +FRAMEWORK_SEARCH_PATHS_ios_Production = $(PRODUCTION_FRAMEWORKS_DIR); > > +FRAMEWORK_SEARCH_PATHS[sdk=macosx*] = $(STAGED_FRAMEWORKS_SEARCH_PATH) $(FRAMEWORK_SEARCH_PATHS); > > And this to use [config=Foo]. Patch v5 will use [config=Production] here. > > Source/WebCore/Configurations/WebCore.xcconfig:62 > > +DYLIB_INSTALL_NAME_BASE = $(DYLIB_INSTALL_NAME_BASE_PLATFORM); > > +DYLIB_INSTALL_NAME_BASE_PLATFORM[sdk=iphoneos*] = $(DYLIB_INSTALL_NAME_BASE_ios); > > +DYLIB_INSTALL_NAME_BASE_PLATFORM[sdk=iphonesimulator*] = $(SDKROOT)$(DYLIB_INSTALL_NAME_BASE_ios); > > +DYLIB_INSTALL_NAME_BASE_PLATFORM[sdk=macosx*] = $(NORMAL_WEBCORE_FRAMEWORKS_DIR); > > Why is the indirection through DYLIB_INSTALL_NAME_BASE_PLATFORM necessary? > Is it because DYLIB_INSTALL_NAME_BASE is set at the same level by > WebCoreTestSupport.xcconfig after it includes this file? This is so DYLIB_INSTALL_NAME_BASE in WebCoreTestSupport.xcconfig will properly override these settings. Using the DYLIB_INSTALL_NAME_BASE[sdk=] would be more specific than what WebCoreTestSupport.xcconfig has, so I'd then have to change DYLIB_INSTALL_NAME_BASE in WebCoreTestSupport.xcconfig to work around this change. Also, I'm not sure whether [sdk=] is more or less specific than [config=], as I could change DYLIB_INSTALL_NAME_BASE in WebCoreTestSupport.xcconfig to use [config=] and WebCore.xcconfig to use [sdk=], but I honestly don't know which one would take precedence. > > Source/WebInspectorUI/Configurations/DebugRelease.xcconfig:7 > > +MACOSX_DEPLOYMENT_TARGET = $(MACOSX_DEPLOYMENT_TARGET_$(PLATFORM_NAME)_$(TARGET_MAC_OS_X_VERSION_MAJOR)); > > Why is this not using [sdk=macosx*]? Workaround for Xcode 5.1.1 bug. > > Source/WebInspectorUI/Configurations/DebugRelease.xcconfig:23 > > +SDKROOT = $(SDKROOT_$(PLATFORM_NAME)_$(USE_INTERNAL_SDK)); > > Ditto. Workaround for Xcode 5.1.1 bug. > > Source/WebInspectorUI/Configurations/Version.xcconfig:10 > > +SYSTEM_VERSION_PREFIX = $(SYSTEM_VERSION_PREFIX_$(PLATFORM_NAME)_$(TARGET_MAC_OS_X_VERSION_MAJOR)); > > Ditto. Workaround for Xcode 5.1.1 bug.
Created attachment 243459 [details] Patch v5
Created attachment 243519 [details] Patch v6 (applies to trunk for EWS testing)
(In reply to comment #13) > Created attachment 243519 [details] > Patch v6 (applies to trunk for EWS testing) And...UMBRELLA_FRAMEWORKS_DIR is not being set properly. Here's the new *.xcconfig file: UMBRELLA_FRAMEWORKS_DIR[config=Debug] = $(UMBRELLA_FRAMEWORKS_DIR_engineering); UMBRELLA_FRAMEWORKS_DIR[config=Release] = $(UMBRELLA_FRAMEWORKS_DIR_engineering); UMBRELLA_FRAMEWORKS_DIR[config=Production][sdk=iphone*] = $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/PrivateFrameworks; UMBRELLA_FRAMEWORKS_DIR[config=Production][sdk=macosx*] = $(SDKROOT)$(UMBRELLA_FRAMEWORKS_DIR_Production_macosx_USE_STAGING_INSTALL_PATH_$(USE_STAGING_INSTALL_PATH)); UMBRELLA_FRAMEWORKS_DIR_Production_macosx_USE_STAGING_INSTALL_PATH_ = $(UMBRELLA_FRAMEWORKS_DIR_Production_macosx_USE_STAGING_INSTALL_PATH_NO); UMBRELLA_FRAMEWORKS_DIR_Production_macosx_USE_STAGING_INSTALL_PATH_NO = $(NEXT_ROOT)$(SYSTEM_LIBRARY_DIR)/Frameworks/WebKit.framework/Versions/A/Frameworks; UMBRELLA_FRAMEWORKS_DIR_Production_macosx_USE_STAGING_INSTALL_PATH_YES = $(NEXT_ROOT)$(SYSTEM_LIBRARY_DIR)/StagedFrameworks/Safari; UMBRELLA_FRAMEWORKS_DIR_engineering = $(BUILT_PRODUCTS_DIR); However, only these variables are set in a Release build: setenv UMBRELLA_FRAMEWORKS_DIR_Production_macosx_USE_STAGING_INSTALL_PATH_ /System/Library/Frameworks/WebKit.framework/Versions/A/Frameworks setenv UMBRELLA_FRAMEWORKS_DIR_Production_macosx_USE_STAGING_INSTALL_PATH_NO /System/Library/Frameworks/WebKit.framework/Versions/A/Frameworks setenv UMBRELLA_FRAMEWORKS_DIR_Production_macosx_USE_STAGING_INSTALL_PATH_YES /System/Library/StagedFrameworks/Safari setenv UMBRELLA_FRAMEWORKS_DIR_engineering /Volumes/Data/EWS/WebKit/WebKitBuild/Release I was expecting Debug/Release builds to use [config=Debug] and [config=Release] since they were the most specific and still matched. But maybe the [config=Production][sdk=macosx*] variable is most specific, doesn't match, and so the variable isn't set? :| Let's try switching it back in Patch v7.
Created attachment 243524 [details] Patch v7 (try to fix UMBRELLA_FRAMEWORKS_DIR in BaseTarget.xcconfig)
Committed r177574: <http://trac.webkit.org/changeset/177574>
Comment on attachment 243524 [details] Patch v7 (try to fix UMBRELLA_FRAMEWORKS_DIR in BaseTarget.xcconfig) Clearing review flag. I landed "Patch v7" since it built on both Mac and iOS.
Re-opened since this is blocked by bug 139821
Created attachment 243574 [details] Patch v8 (remove all uses of [config=])
Committed r177621: <http://trac.webkit.org/changeset/177621>
Comment on attachment 243574 [details] Patch v8 (remove all uses of [config=]) Patch v8 landed as r177621. Clearing review flag.