Bug 224785 – Build ANGLE dylib into WK_OVERRIDE_FRAMEWORKS_DIR in builds that use it

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Build ANGLE dylib into WK_OVERRIDE_FRAMEWORKS_DIR in builds that use it

[Alex Christensen]

Created attachment 426481 [details]
Patch

[Alex Christensen]

<rdar://problem/76641662>

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE

[Alex Christensen]

Created attachment 426483 [details]
Patch

[Alex Christensen]

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.

[Alexey Proskuryakov]

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?

[Alex Christensen]

(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

[Alexey Proskuryakov]

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?

[Alex Christensen]

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.

[Alex Christensen]

(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)

[Alexey Proskuryakov]

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.

[EWS]

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].