| Summary: | Build ANGLE dylib into WK_OVERRIDE_FRAMEWORKS_DIR in builds that use it | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||
| Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ap, dino, ews-watchlist, graouts, kkinnunen, kondapallykalyan, thorton, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=224602 | ||||||||
| Attachments: |
|
||||||||
|
Description
Alex Christensen
2021-04-19 15:29:14 PDT
Created attachment 426481 [details]
Patch
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE Created attachment 426483 [details]
Patch
I confirmed that this puts it in the same location as the libwebrtc.dylib in this build, and before this change it was in an obviously wrong place. Comment on attachment 426483 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426483&action=review > Source/ThirdParty/ANGLE/Configurations/ANGLE-dynamic.xcconfig:22 > +INSTALL_PATH_USE_ALTERNATE_FRAMEWORKS_DIR_NO = $(INSTALL_PATH_USE_OVERRIDE_FRAMEWORKS_DIR_$(WK_USE_OVERRIDE_FRAMEWORKS_DIR)); > +INSTALL_PATH_USE_OVERRIDE_FRAMEWORKS_DIR_NO = $(NORMAL_WEBCORE_FRAMEWORKS_DIR)/WebCore.framework/Versions/A/Frameworks; It seems really surprising that libwebrtc doesn't use WK_USE_ALTERNATE_FRAMEWORKS_DIR, and ANGLE will be using both WK_USE_ALTERNATE_FRAMEWORKS_DIR and WK_USE_OVERRIDE_FRAMEWORKS_DIR. Are they intentionally different in some way? > Source/ThirdParty/ANGLE/Configurations/ANGLE-dynamic.xcconfig:26 > DYLIB_INSTALL_NAME_BASE[sdk=macosx*] = $(DYLIB_INSTALL_NAME_BASE_USE_ALTERNATE_FRAMEWORKS_DIR_$(WK_USE_ALTERNATE_FRAMEWORKS_DIR)); Does this also need to be updated with WK_USE_OVERRIDE_FRAMEWORKS_DIR? (In reply to Alexey Proskuryakov from comment #6) > Comment on attachment 426483 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=426483&action=review > > > Source/ThirdParty/ANGLE/Configurations/ANGLE-dynamic.xcconfig:22 > > +INSTALL_PATH_USE_ALTERNATE_FRAMEWORKS_DIR_NO = $(INSTALL_PATH_USE_OVERRIDE_FRAMEWORKS_DIR_$(WK_USE_OVERRIDE_FRAMEWORKS_DIR)); > > +INSTALL_PATH_USE_OVERRIDE_FRAMEWORKS_DIR_NO = $(NORMAL_WEBCORE_FRAMEWORKS_DIR)/WebCore.framework/Versions/A/Frameworks; > > It seems really surprising that libwebrtc doesn't use > WK_USE_ALTERNATE_FRAMEWORKS_DIR, and ANGLE will be using both > WK_USE_ALTERNATE_FRAMEWORKS_DIR and WK_USE_OVERRIDE_FRAMEWORKS_DIR. > > Are they intentionally different in some way? WK_ALTERNATE_FRAMEWORKS_DIR is used in the Catalyst build where WebGL is supported but WebRTC is not. If we decide to enable WebRTC for Catalyst, then we will need to start using WK_ALTERNATE_FRAMEWORKS_DIR in the libwebrtc build. > > > Source/ThirdParty/ANGLE/Configurations/ANGLE-dynamic.xcconfig:26 > > DYLIB_INSTALL_NAME_BASE[sdk=macosx*] = $(DYLIB_INSTALL_NAME_BASE_USE_ALTERNATE_FRAMEWORKS_DIR_$(WK_USE_ALTERNATE_FRAMEWORKS_DIR)); > > Does this also need to be updated with WK_USE_OVERRIDE_FRAMEWORKS_DIR? It does not. This line is effectively the branch point between Catalyst, which uses WK_ALTERNATE_FRAMEWORKS_DIR, and non-Catalyst builds which do not. This patch introduces another branch point between the operating system builds, which do not use WK_OVERRIDE_FRAMEWORKS_DIR, from the Safari update builds on older macOSes, which do use WK_OVERRIDE_FRAMEWORKS_DIR Comment on attachment 426483 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426483&action=review >>> Source/ThirdParty/ANGLE/Configurations/ANGLE-dynamic.xcconfig:22 >>> +INSTALL_PATH_USE_OVERRIDE_FRAMEWORKS_DIR_NO = $(NORMAL_WEBCORE_FRAMEWORKS_DIR)/WebCore.framework/Versions/A/Frameworks; >> >> It seems really surprising that libwebrtc doesn't use WK_USE_ALTERNATE_FRAMEWORKS_DIR, and ANGLE will be using both WK_USE_ALTERNATE_FRAMEWORKS_DIR and WK_USE_OVERRIDE_FRAMEWORKS_DIR. >> >> Are they intentionally different in some way? > > WK_ALTERNATE_FRAMEWORKS_DIR is used in the Catalyst build where WebGL is supported but WebRTC is not. If we decide to enable WebRTC for Catalyst, then we will need to start using WK_ALTERNATE_FRAMEWORKS_DIR in the libwebrtc build. OK. Looks like most of our projects implement the logic for this differently, but the location of WebCore is a special level of crazy. >>> Source/ThirdParty/ANGLE/Configurations/ANGLE-dynamic.xcconfig:26 >>> DYLIB_INSTALL_NAME_BASE[sdk=macosx*] = $(DYLIB_INSTALL_NAME_BASE_USE_ALTERNATE_FRAMEWORKS_DIR_$(WK_USE_ALTERNATE_FRAMEWORKS_DIR)); >> >> Does this also need to be updated with WK_USE_OVERRIDE_FRAMEWORKS_DIR? > > It does not. This line is effectively the branch point between Catalyst, which uses WK_ALTERNATE_FRAMEWORKS_DIR, and non-Catalyst builds which do not. This patch introduces another branch point between the operating system builds, which do not use > WK_OVERRIDE_FRAMEWORKS_DIR, from the Safari update builds on older macOSes, which do use WK_OVERRIDE_FRAMEWORKS_DIR Right, but why do we not need a different DYLIB_INSTALL_NAME_BASE for Safari update builds on older macOSes, given that they are installed at a different location? Libwebrtc needs it. Although it's the only project that does. To be clear, I have no clue about what WK_RELOCATABLE_FRAMEWORKS is, so maybe it's magically happening through it? Comment on attachment 426483 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426483&action=review >>>> Source/ThirdParty/ANGLE/Configurations/ANGLE-dynamic.xcconfig:26 >>>> DYLIB_INSTALL_NAME_BASE[sdk=macosx*] = $(DYLIB_INSTALL_NAME_BASE_USE_ALTERNATE_FRAMEWORKS_DIR_$(WK_USE_ALTERNATE_FRAMEWORKS_DIR)); >>> >>> Does this also need to be updated with WK_USE_OVERRIDE_FRAMEWORKS_DIR? >> >> It does not. This line is effectively the branch point between Catalyst, which uses WK_ALTERNATE_FRAMEWORKS_DIR, and non-Catalyst builds which do not. This patch introduces another branch point between the operating system builds, which do not use >> WK_OVERRIDE_FRAMEWORKS_DIR, from the Safari update builds on older macOSes, which do use WK_OVERRIDE_FRAMEWORKS_DIR > > Right, but why do we not need a different DYLIB_INSTALL_NAME_BASE for Safari update builds on older macOSes, given that they are installed at a different location? Libwebrtc needs it. Although it's the only project that does. To be clear, I have no clue about what WK_RELOCATABLE_FRAMEWORKS is, so maybe it's magically happening through it? Ah, now I see what you were talking about. I believe WK_RELOCATABLE_FRAMEWORKS just does the right thing, but I'm not 100% sure. If I'm wrong, then I'll get another bug saying WebGL doesn't work in some configuration and I'll fix it, but I'm sure that I currently have the bug I do have about the linker not finding the dylib, and the change above should fix that. (In reply to Alex Christensen from comment #9) > If I'm wrong, then I'll get another bug saying WebGL doesn't work in some > configuration and I'll fix it. (I'm planning to check after running this fix through the internal build system) Comment on attachment 426483 [details]
Patch
I think that our ability to verify through CI is limited, but if this seems like a progression, even if incomplete.
Committed r276286 (236768@main): <https://commits.webkit.org/236768@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426483 [details]. |