[MacCatalyst] ANGLE is putting its public headers in the wrong place
Created attachment 446340 [details] Patch
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Comment on attachment 446340 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446340&action=review > Source/ThirdParty/ANGLE/ChangeLog:3 > + [MacCatalyst] ANGLE is putting its public headers in the wrong place It looks like both native and macCatalyst builds will be producing the same SDK header paths with this patch. Which won’t work, projects shouldn’t be overwriting files that other projects already created. To be clear, I’m talking about final SDK paths here, not ones that you posted in the ChangeLog below. Am I missing something?
Looking at WTF, I see that production builds put iOSMac SDK content in /System/iOSSupport/usr/local, so there is no overlap.
Created attachment 446425 [details] Patch
Comment on attachment 446425 [details] Patch I'm surprised we didn't have to update WebCore's BaseTarget.xcconfig too
I think Alexey's point is right - this is the wrong solution. I need to match what WTF does.
Comment on attachment 446425 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446425&action=review > Source/WebKit/Configurations/BaseTarget.xcconfig:51 > +ANGLE_HEADER_ALTERNATE_SEARCH_PATHS = $(ANGLE_HEADER_ALTERNATE_SEARCH_PATHS_$(SDK_VARIANT)); > +ANGLE_HEADER_ALTERNATE_SEARCH_PATHS_iosmac = $(BUILT_PRODUCTS_DIR)$(WK_ALTERNATE_FRAMEWORKS_DIR)/usr/local/include Do we need the same for WTF? > Source/WebKit/Configurations/BaseTarget.xcconfig:53 > +HEADER_SEARCH_PATHS = $(BUILT_PRODUCTS_DIR)/usr/local/include "$(WEBCORE_PRIVATE_HEADERS_DIR)/ForwardingHeaders" $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKit "$(BUILT_PRODUCTS_DIR)/usr/local/include/pal/graphics/WebGPU" $(WEBKITADDITIONS_HEADER_SEARCH_PATHS) $(LIBWEBRTC_HEADER_SEARCH_PATHS) $(ANGLE_HEADER_ALTERNATE_SEARCH_PATHS) $(SRCROOT) $(HEADER_SEARCH_PATHS); Should ANGLE_HEADER_ALTERNATE_SEARCH_PATHS come first, to override the versions in /usr/local?
(In reply to Dean Jackson from comment #6) > Comment on attachment 446425 [details] > Patch > > I'm surprised we didn't have to update WebCore's BaseTarget.xcconfig too WebCore already has this (Configurations/Webcore) ANGLE_HEADER_SEARCH_PATHS = $(BUILT_PRODUCTS_DIR)$(WK_ALTERNATE_FRAMEWORKS_DIR)/usr/local/include/ $(SDKROOT)$(WK_ALTERNATE_WEBKIT_SDK_PATH)/usr/local/include/;
(In reply to Alexey Proskuryakov from comment #8) > Comment on attachment 446425 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=446425&action=review > > > Source/WebKit/Configurations/BaseTarget.xcconfig:51 > > +ANGLE_HEADER_ALTERNATE_SEARCH_PATHS = $(ANGLE_HEADER_ALTERNATE_SEARCH_PATHS_$(SDK_VARIANT)); > > +ANGLE_HEADER_ALTERNATE_SEARCH_PATHS_iosmac = $(BUILT_PRODUCTS_DIR)$(WK_ALTERNATE_FRAMEWORKS_DIR)/usr/local/include > > Do we need the same for WTF? > > > Source/WebKit/Configurations/BaseTarget.xcconfig:53 > > +HEADER_SEARCH_PATHS = $(BUILT_PRODUCTS_DIR)/usr/local/include "$(WEBCORE_PRIVATE_HEADERS_DIR)/ForwardingHeaders" $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKit "$(BUILT_PRODUCTS_DIR)/usr/local/include/pal/graphics/WebGPU" $(WEBKITADDITIONS_HEADER_SEARCH_PATHS) $(LIBWEBRTC_HEADER_SEARCH_PATHS) $(ANGLE_HEADER_ALTERNATE_SEARCH_PATHS) $(SRCROOT) $(HEADER_SEARCH_PATHS); > > Should ANGLE_HEADER_ALTERNATE_SEARCH_PATHS come first, to override the > versions in /usr/local? Good call, yes.
Comment on attachment 446425 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446425&action=review >> Source/WebKit/Configurations/BaseTarget.xcconfig:51 >> +ANGLE_HEADER_ALTERNATE_SEARCH_PATHS_iosmac = $(BUILT_PRODUCTS_DIR)$(WK_ALTERNATE_FRAMEWORKS_DIR)/usr/local/include > > Do we need the same for WTF? Given that we effectively apply to to everything, probably shouldn't have ANGLE in the name anyway, just ALTERNATE_HEADER_SEARCH_PATHS.
Created attachment 446447 [details] Patch
Comment on attachment 446447 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446447&action=review > Source/WebKit/Configurations/BaseTarget.xcconfig:53 > +HEADER_SEARCH_PATHS = $(ALTERNATE_HEADER_SEARCH_PATHS) $(BUILT_PRODUCTS_DIR)/usr/local/include "$(WEBCORE_PRIVATE_HEADERS_DIR)/ForwardingHeaders" $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKit "$(BUILT_PRODUCTS_DIR)/usr/local/include/pal/graphics/WebGPU" $(WEBKITADDITIONS_HEADER_SEARCH_PATHS) $(LIBWEBRTC_HEADER_SEARCH_PATHS) $(SRCROOT) $(HEADER_SEARCH_PATHS); Please don't add two spaces on this line. I think that I was not precise enough. ALTERNATE_HEADER_SEARCH_PATHS should probably come after SRCROOT still, but before HEADER_SEARCH_PATHS. The current order seems very inconsistent, with it being the very first. > Tools/TestWebKitAPI/Configurations/Base.xcconfig:40 > +HEADER_SEARCH_PATHS = $(ALTERNATE_HEADER_SEARCH_PATHS) $(BUILT_PRODUCTS_DIR)/usr/local/include "$(WEBCORE_PRIVATE_HEADERS_DIR)/ForwardingHeaders" $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKit "$(BUILT_PRODUCTS_DIR)/usr/local/include/pal/graphics/WebGPU" $(WEBKITADDITIONS_HEADER_SEARCH_PATHS) $(LIBWEBRTC_HEADER_SEARCH_PATHS) $(SRCROOT) $(HEADER_SEARCH_PATHS); Ditto.
Created attachment 446452 [details] Patch
Comment on attachment 446452 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446452&action=review > Source/WebKit/Configurations/BaseTarget.xcconfig:53 > +HEADER_SEARCH_PATHS = $(BUILT_PRODUCTS_DIR)/usr/local/include "$(WEBCORE_PRIVATE_HEADERS_DIR)/ForwardingHeaders" $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKit "$(BUILT_PRODUCTS_DIR)/usr/local/include/pal/graphics/WebGPU" $(WEBKITADDITIONS_HEADER_SEARCH_PATHS) $(LIBWEBRTC_HEADER_SEARCH_PATHS) $(SRCROOT) $(ALTERNATE_HEADER_SEARCH_PATHS) $(HEADER_SEARCH_PATHS); I'm sorry for my carelessness in review. ALTERNATE_HEADER_SEARCH_PATHS is actually parallel to $(BUILT_PRODUCTS_DIR)/usr/local/include, not to the original value of $(HEADER_SEARCH_PATHS), so it should be first in fact?
Created attachment 446457 [details] Patch
Created attachment 446461 [details] Patch
Comment on attachment 446461 [details] Patch Looks good to me! I'm going to clear your r? flag, because otherwise commit queue would fail - you already have Dean as the reviewer in ChangeLogs.
Committed r286757 (244998@main): <https://commits.webkit.org/244998@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 446461 [details].
<rdar://problem/86247153>