224785 2021-04-19 15:29:14 -0700 Build ANGLE dylib into WK_OVERRIDE_FRAMEWORKS_DIR in builds that use it 2021-04-20 09:55:50 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=224602 InRadar P2 Normal --- 1 achristensen achristensen ap dino ews-watchlist graouts kkinnunen kondapallykalyan thorton webkit-bug-importer oldest_to_newest 1752009 0 achristensen 2021-04-19 15:29:14 -0700 Build ANGLE dylib into WK_OVERRIDE_FRAMEWORKS_DIR in builds that use it 1752010 1 426481 achristensen 2021-04-19 15:30:34 -0700 Created attachment 426481 Patch 1752011 2 achristensen 2021-04-19 15:30:37 -0700 <rdar://problem/76641662> 1752012 3 ews-watchlist 2021-04-19 15:31:24 -0700 Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE 1752021 4 426483 achristensen 2021-04-19 15:48:00 -0700 Created attachment 426483 Patch 1752022 5 achristensen 2021-04-19 15:48:49 -0700 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. 1752053 6 426483 ap 2021-04-19 16:58:30 -0700 Comment on attachment 426483 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? 1752055 7 achristensen 2021-04-19 17:04:09 -0700 (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 1752071 8 426483 ap 2021-04-19 17:40:02 -0700 Comment on attachment 426483 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? 1752117 9 426483 achristensen 2021-04-19 20:23:59 -0700 Comment on attachment 426483 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. 1752121 10 achristensen 2021-04-19 20:41:22 -0700 (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) 1752133 11 426483 ap 2021-04-19 21:50:02 -0700 Comment on attachment 426483 Patch I think that our ability to verify through CI is limited, but if this seems like a progression, even if incomplete. 1752146 12 ews-feeder 2021-04-19 22:19:38 -0700 Committed r276286 (236768@main): <https://commits.webkit.org/236768@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426483. 426481 2021-04-19 15:30:34 -0700 2021-04-19 15:47:58 -0700 Patch bug-224785-20210419153033.patch text/plain 2196 achristensen U3VidmVyc2lvbiBSZXZpc2lvbjogMjc2MjcwCmRpZmYgLS1naXQgYS9Tb3VyY2UvVGhpcmRQYXJ0 eS9BTkdMRS9DaGFuZ2VMb2cgYi9Tb3VyY2UvVGhpcmRQYXJ0eS9BTkdMRS9DaGFuZ2VMb2cKaW5k ZXggNDRlNWU3NjFjYjk0NjU2YWQ1MTZkN2ZmM2Q2ZjQyZmE4NmQ3YjRiYy4uMDBlZmEzNzgwZWI5 MTUwYjc2NzkzNzc5YWM0YTJkNjVhMzY0YzkzYSAxMDA2NDQKLS0tIGEvU291cmNlL1RoaXJkUGFy dHkvQU5HTEUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9UaGlyZFBhcnR5L0FOR0xFL0NoYW5nZUxv ZwpAQCAtMSwzICsxLDEzIEBACisyMDIxLTA0LTE5ICBBbGV4IENocmlzdGVuc2VuICA8YWNocmlz dGVuc2VuQHdlYmtpdC5vcmc+CisKKyAgICAgICAgQnVpbGQgQU5HTEUgZHlsaWIgaW50byBXS19P VkVSUklERV9GUkFNRVdPUktTX0RJUiBpbiBidWlsZHMgdGhhdCB1c2UgaXQKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIyNDc4NQorICAgICAgICA8cmRh cjovLzc2NjQxNjYyPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgICogQ29uZmlndXJhdGlvbnMvQU5HTEUtZHluYW1pYy54Y2NvbmZpZzoKKwogMjAyMS0w NC0xNiAgS3lsZSBQaWRkaW5ndG9uICA8a3BpZGRpbmd0b25AYXBwbGUuY29tPgogCiAgICAgICAg IE1ldGFsLUFOR0xFOiBTaGFyZWQgbWVtb3J5IHRleHR1cmUgdGVzdHMgZmFpbGluZyBpbiBpT1Mg U2ltdWxhdG9yCmRpZmYgLS1naXQgYS9Tb3VyY2UvVGhpcmRQYXJ0eS9BTkdMRS9Db25maWd1cmF0 aW9ucy9BTkdMRS1keW5hbWljLnhjY29uZmlnIGIvU291cmNlL1RoaXJkUGFydHkvQU5HTEUvQ29u ZmlndXJhdGlvbnMvQU5HTEUtZHluYW1pYy54Y2NvbmZpZwppbmRleCA3MTA1NWE2NGUzZjM5NDVj MTU4OTkxZjA0ZjE1YzEwYzM5ZDcwNjlhLi4zYTYyMmNkNzliZmVjNjQ3MTg2NTRmMWZhOGJkOGRi MmYwODg2ZDdjIDEwMDY0NAotLS0gYS9Tb3VyY2UvVGhpcmRQYXJ0eS9BTkdMRS9Db25maWd1cmF0 aW9ucy9BTkdMRS1keW5hbWljLnhjY29uZmlnCisrKyBiL1NvdXJjZS9UaGlyZFBhcnR5L0FOR0xF L0NvbmZpZ3VyYXRpb25zL0FOR0xFLWR5bmFtaWMueGNjb25maWcKQEAgLTE4LDcgKzE4LDkgQEAg QU5HTEVfU1RBVElDX0xJQl9PVEhFUl9MREZMQUdTID0gLVdsLC1mb3JjZV9sb2FkLCIkKEJVSUxU X1BST0RVQ1RTX0RJUikvbGliQU5HTEUKIAogSU5TVEFMTF9QQVRIID0gJChOT1JNQUxfV0VCQ09S RV9GUkFNRVdPUktTX0RJUikvV2ViQ29yZS5mcmFtZXdvcmsvRnJhbWV3b3JrczsKIElOU1RBTExf UEFUSFtzZGs9bWFjb3N4Kl0gPSAkKElOU1RBTExfUEFUSF9VU0VfQUxURVJOQVRFX0ZSQU1FV09S S1NfRElSXyQoV0tfVVNFX0FMVEVSTkFURV9GUkFNRVdPUktTX0RJUikpOwotSU5TVEFMTF9QQVRI X1VTRV9BTFRFUk5BVEVfRlJBTUVXT1JLU19ESVJfTk8gPSAkKE5PUk1BTF9XRUJDT1JFX0ZSQU1F V09SS1NfRElSKS9XZWJDb3JlLmZyYW1ld29yay9WZXJzaW9ucy9BL0ZyYW1ld29ya3M7CitJTlNU QUxMX1BBVEhfVVNFX0FMVEVSTkFURV9GUkFNRVdPUktTX0RJUl9OTyA9ICQoSU5TVEFMTF9QQVRI X1VTRV9PVkVSUklERV9GUkFNRVdPUktTX0RJUl8kKFdLX1VTRV9PVkVSUklERV9GUkFNRVdPUktT X0RJUikpOworSU5TVEFMTF9QQVRIX1VTRV9PVkVSUklERV9GUkFNRVdPUktTX0RJUl9OTyA9ICQo Tk9STUFMX1dFQkNPUkVfRlJBTUVXT1JLU19ESVIpL1dlYkNvcmUuZnJhbWV3b3JrL1ZlcnNpb25z L0EvRnJhbWV3b3JrczsKK0lOU1RBTExfUEFUSF9VU0VfT1ZFUlJJREVfRlJBTUVXT1JLU19ESVJf WUVTID0gJChXS19PVkVSUklERV9GUkFNRVdPUktTX0RJUikvV2ViQ29yZS5mcmFtZXdvcmsvVmVy c2lvbnMvQS9GcmFtZXdvcmtzOwogSU5TVEFMTF9QQVRIX1VTRV9BTFRFUk5BVEVfRlJBTUVXT1JL U19ESVJfWUVTID0gJChXS19BTFRFUk5BVEVfRlJBTUVXT1JLU19ESVIpLyQoU1lTVEVNX0xJQlJB UllfRElSKS9Qcml2YXRlRnJhbWV3b3Jrcy9XZWJDb3JlLmZyYW1ld29yay9WZXJzaW9ucy9BL0Zy YW1ld29ya3M7CiAKIERZTElCX0lOU1RBTExfTkFNRV9CQVNFW3Nkaz1tYWNvc3gqXSA9ICQoRFlM SUJfSU5TVEFMTF9OQU1FX0JBU0VfVVNFX0FMVEVSTkFURV9GUkFNRVdPUktTX0RJUl8kKFdLX1VT RV9BTFRFUk5BVEVfRlJBTUVXT1JLU19ESVIpKTsK 426483 2021-04-19 15:48:00 -0700 2021-04-19 22:19:39 -0700 Patch bug-224785-20210419154759.patch text/plain 2156 achristensen U3VidmVyc2lvbiBSZXZpc2lvbjogMjc2MjcwCmRpZmYgLS1naXQgYS9Tb3VyY2UvVGhpcmRQYXJ0 eS9BTkdMRS9DaGFuZ2VMb2cgYi9Tb3VyY2UvVGhpcmRQYXJ0eS9BTkdMRS9DaGFuZ2VMb2cKaW5k ZXggNDRlNWU3NjFjYjk0NjU2YWQ1MTZkN2ZmM2Q2ZjQyZmE4NmQ3YjRiYy4uMDBlZmEzNzgwZWI5 MTUwYjc2NzkzNzc5YWM0YTJkNjVhMzY0YzkzYSAxMDA2NDQKLS0tIGEvU291cmNlL1RoaXJkUGFy dHkvQU5HTEUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9UaGlyZFBhcnR5L0FOR0xFL0NoYW5nZUxv ZwpAQCAtMSwzICsxLDEzIEBACisyMDIxLTA0LTE5ICBBbGV4IENocmlzdGVuc2VuICA8YWNocmlz dGVuc2VuQHdlYmtpdC5vcmc+CisKKyAgICAgICAgQnVpbGQgQU5HTEUgZHlsaWIgaW50byBXS19P VkVSUklERV9GUkFNRVdPUktTX0RJUiBpbiBidWlsZHMgdGhhdCB1c2UgaXQKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIyNDc4NQorICAgICAgICA8cmRh cjovLzc2NjQxNjYyPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgICogQ29uZmlndXJhdGlvbnMvQU5HTEUtZHluYW1pYy54Y2NvbmZpZzoKKwogMjAyMS0w NC0xNiAgS3lsZSBQaWRkaW5ndG9uICA8a3BpZGRpbmd0b25AYXBwbGUuY29tPgogCiAgICAgICAg IE1ldGFsLUFOR0xFOiBTaGFyZWQgbWVtb3J5IHRleHR1cmUgdGVzdHMgZmFpbGluZyBpbiBpT1Mg U2ltdWxhdG9yCmRpZmYgLS1naXQgYS9Tb3VyY2UvVGhpcmRQYXJ0eS9BTkdMRS9Db25maWd1cmF0 aW9ucy9BTkdMRS1keW5hbWljLnhjY29uZmlnIGIvU291cmNlL1RoaXJkUGFydHkvQU5HTEUvQ29u ZmlndXJhdGlvbnMvQU5HTEUtZHluYW1pYy54Y2NvbmZpZwppbmRleCA3MTA1NWE2NGUzZjM5NDVj MTU4OTkxZjA0ZjE1YzEwYzM5ZDcwNjlhLi4zNzdlN2MzZWNjOTRmNTZiNTVjM2JjMjYxYmVhNDgw YWQyNzIxYjAyIDEwMDY0NAotLS0gYS9Tb3VyY2UvVGhpcmRQYXJ0eS9BTkdMRS9Db25maWd1cmF0 aW9ucy9BTkdMRS1keW5hbWljLnhjY29uZmlnCisrKyBiL1NvdXJjZS9UaGlyZFBhcnR5L0FOR0xF L0NvbmZpZ3VyYXRpb25zL0FOR0xFLWR5bmFtaWMueGNjb25maWcKQEAgLTE4LDcgKzE4LDkgQEAg QU5HTEVfU1RBVElDX0xJQl9PVEhFUl9MREZMQUdTID0gLVdsLC1mb3JjZV9sb2FkLCIkKEJVSUxU X1BST0RVQ1RTX0RJUikvbGliQU5HTEUKIAogSU5TVEFMTF9QQVRIID0gJChOT1JNQUxfV0VCQ09S RV9GUkFNRVdPUktTX0RJUikvV2ViQ29yZS5mcmFtZXdvcmsvRnJhbWV3b3JrczsKIElOU1RBTExf UEFUSFtzZGs9bWFjb3N4Kl0gPSAkKElOU1RBTExfUEFUSF9VU0VfQUxURVJOQVRFX0ZSQU1FV09S S1NfRElSXyQoV0tfVVNFX0FMVEVSTkFURV9GUkFNRVdPUktTX0RJUikpOwotSU5TVEFMTF9QQVRI X1VTRV9BTFRFUk5BVEVfRlJBTUVXT1JLU19ESVJfTk8gPSAkKE5PUk1BTF9XRUJDT1JFX0ZSQU1F V09SS1NfRElSKS9XZWJDb3JlLmZyYW1ld29yay9WZXJzaW9ucy9BL0ZyYW1ld29ya3M7CitJTlNU QUxMX1BBVEhfVVNFX0FMVEVSTkFURV9GUkFNRVdPUktTX0RJUl9OTyA9ICQoSU5TVEFMTF9QQVRI X1VTRV9PVkVSUklERV9GUkFNRVdPUktTX0RJUl8kKFdLX1VTRV9PVkVSUklERV9GUkFNRVdPUktT X0RJUikpOworSU5TVEFMTF9QQVRIX1VTRV9PVkVSUklERV9GUkFNRVdPUktTX0RJUl9OTyA9ICQo Tk9STUFMX1dFQkNPUkVfRlJBTUVXT1JLU19ESVIpL1dlYkNvcmUuZnJhbWV3b3JrL1ZlcnNpb25z L0EvRnJhbWV3b3JrczsKK0lOU1RBTExfUEFUSF9VU0VfT1ZFUlJJREVfRlJBTUVXT1JLU19ESVJf WUVTID0gJChXS19PVkVSUklERV9GUkFNRVdPUktTX0RJUik7CiBJTlNUQUxMX1BBVEhfVVNFX0FM VEVSTkFURV9GUkFNRVdPUktTX0RJUl9ZRVMgPSAkKFdLX0FMVEVSTkFURV9GUkFNRVdPUktTX0RJ UikvJChTWVNURU1fTElCUkFSWV9ESVIpL1ByaXZhdGVGcmFtZXdvcmtzL1dlYkNvcmUuZnJhbWV3 b3JrL1ZlcnNpb25zL0EvRnJhbWV3b3JrczsKIAogRFlMSUJfSU5TVEFMTF9OQU1FX0JBU0Vbc2Rr PW1hY29zeCpdID0gJChEWUxJQl9JTlNUQUxMX05BTUVfQkFTRV9VU0VfQUxURVJOQVRFX0ZSQU1F V09SS1NfRElSXyQoV0tfVVNFX0FMVEVSTkFURV9GUkFNRVdPUktTX0RJUikpOwo=