Build ANGLE as a dynamic library
rdar://57349384
Created attachment 384524 [details] Patch
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Created attachment 384527 [details] Patch
Comment on attachment 384527 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384527&action=review > Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:79 > +// DEBUG_DEFINES, GCC_OPTIMIZATION_LEVEL, STRIP_INSTALLED_PRODUCT and DEAD_CODE_STRIPPING vary between the debug and normal variants. > +// We set up the values for each variant here, and have the Debug configuration in the Xcode project use the _debug variant. Is this comment true? I don’t see anything in the Xcode project that defines DEBUG_DEFINES, STRIP_INSTALLED_PRODUCT, or DEAD_CODE_STRIPPING, and I don’t think the Debug configuration builds the “debug” variant (that is, I believe it builds the “normal” variant). > Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:96 > +OTHER_CFLAGS = $(ASAN_OTHER_CFLAGS) -fvisibility=hidden; I think this is better specified with the dedicated build setting GCC_SYMBOLS_PRIVATE_EXTERN. > Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:97 > +OTHER_CPLUSPLUSFLAGS = $(ASAN_OTHER_CPLUSPLUSFLAGS) -fvisibility=hidden; You seem to have left this setting in the Xcode project file, where I think it takes precedence over this. Either way it’s probably best to just use GCC_SYMBOLS_PRIVATE_EXTERN. > Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:98 > +OTHER_LDFLAGS = $(ASAN_OTHER_LDFLAGS) $(ANGLE_OTHER_LDFLAGS) -fvisibility=hidden; Similar comment here. > Source/WebCore/Configurations/WebCore.xcconfig:97 > +WK_ANGLE_LDFLAGS = -weak-lANGLE; What guarantees that symbols from ANGLE aren’t dereferenced at runtime when it’s not present?
Created attachment 384530 [details] Patch
Comment on attachment 384527 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384527&action=review >> Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:79 >> +// We set up the values for each variant here, and have the Debug configuration in the Xcode project use the _debug variant. > > Is this comment true? I don’t see anything in the Xcode project that defines DEBUG_DEFINES, STRIP_INSTALLED_PRODUCT, or DEAD_CODE_STRIPPING, and I don’t think the Debug configuration builds the “debug” variant (that is, I believe it builds the “normal” variant). You're right. This was copied from another .xcconfig file - I removed it in the subsequent patch. >> Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:96 >> +OTHER_CFLAGS = $(ASAN_OTHER_CFLAGS) -fvisibility=hidden; > > I think this is better specified with the dedicated build setting GCC_SYMBOLS_PRIVATE_EXTERN. Done. >> Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:97 >> +OTHER_CPLUSPLUSFLAGS = $(ASAN_OTHER_CPLUSPLUSFLAGS) -fvisibility=hidden; > > You seem to have left this setting in the Xcode project file, where I think it takes precedence over this. Either way it’s probably best to just use GCC_SYMBOLS_PRIVATE_EXTERN. Removed from the Xcode project file. >> Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:98 >> +OTHER_LDFLAGS = $(ASAN_OTHER_LDFLAGS) $(ANGLE_OTHER_LDFLAGS) -fvisibility=hidden; > > Similar comment here. Also done. >> Source/WebCore/Configurations/WebCore.xcconfig:97 >> +WK_ANGLE_LDFLAGS = -weak-lANGLE; > > What guarantees that symbols from ANGLE aren’t dereferenced at runtime when it’s not present? Nothing. libwebrtc does the same thing. What should I do? Would you prefer I explicitly dlopen?
Created attachment 384531 [details] Patch
(In reply to Dean Jackson from comment #7) > Comment on attachment 384527 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=384527&action=review > > >> Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:79 > >> +// We set up the values for each variant here, and have the Debug configuration in the Xcode project use the _debug variant. > > > > Is this comment true? I don’t see anything in the Xcode project that defines DEBUG_DEFINES, STRIP_INSTALLED_PRODUCT, or DEAD_CODE_STRIPPING, and I don’t think the Debug configuration builds the “debug” variant (that is, I believe it builds the “normal” variant). > > You're right. This was copied from another .xcconfig file - I removed it in > the subsequent patch. I can’t tell if anything else guarantees that it build with the right values of these build settings for Debug, Release, and Production, but I guess it’s no different than before. > > >> Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:96 > >> +OTHER_CFLAGS = $(ASAN_OTHER_CFLAGS) -fvisibility=hidden; > > > > I think this is better specified with the dedicated build setting GCC_SYMBOLS_PRIVATE_EXTERN. > > Done. Seems like this was already present (just above where you added it). > > >> Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:97 > >> +OTHER_CPLUSPLUSFLAGS = $(ASAN_OTHER_CPLUSPLUSFLAGS) -fvisibility=hidden; > > > > You seem to have left this setting in the Xcode project file, where I think it takes precedence over this. Either way it’s probably best to just use GCC_SYMBOLS_PRIVATE_EXTERN. > > Removed from the Xcode project file. > > >> Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:98 > >> +OTHER_LDFLAGS = $(ASAN_OTHER_LDFLAGS) $(ANGLE_OTHER_LDFLAGS) -fvisibility=hidden; > > > > Similar comment here. > > Also done. > > >> Source/WebCore/Configurations/WebCore.xcconfig:97 > >> +WK_ANGLE_LDFLAGS = -weak-lANGLE; > > > > What guarantees that symbols from ANGLE aren’t dereferenced at runtime when it’s not present? > > Nothing. libwebrtc does the same thing. Not quite. In platform/mediastream/libwebrtc/LibWebRTCProviderCocoa.cpp I see LibWebRTCProvider::webRTCAvailable checking whether the library is loaded; presumably all other uses of the library are gated on that function. The question is whether something similar needs to be done with ANGLE. > Would you prefer I explicitly dlopen? Absolutely not.
(In reply to mitz from comment #9) > > > I think this is better specified with the dedicated build setting GCC_SYMBOLS_PRIVATE_EXTERN. > > > > Done. > > Seems like this was already present (just above where you added it). Yes. > > >> +WK_ANGLE_LDFLAGS = -weak-lANGLE; > > > > > > What guarantees that symbols from ANGLE aren’t dereferenced at runtime when it’s not present? > > > > Nothing. libwebrtc does the same thing. > > Not quite. In platform/mediastream/libwebrtc/LibWebRTCProviderCocoa.cpp I > see LibWebRTCProvider::webRTCAvailable checking whether the library is > loaded; presumably all other uses of the library are gated on that function. > The question is whether something similar needs to be done with ANGLE. I should. Thank you for noticing! > > Would you prefer I explicitly dlopen? > > Absolutely not. OK.
Created attachment 384546 [details] Patch
Created attachment 384547 [details] Patch
Created attachment 384548 [details] Patch
Created attachment 384551 [details] Patch
Finally got all the ports building. Also tested with the Apple-internal build system.
Comment on attachment 384551 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384551&action=review > Source/ThirdParty/ANGLE/Configurations/ANGLE.xcconfig:10 > +ANGLE_OTHER_LDFLAGS_iphonesimulator = -framework OpenGLES; Why no IOSurface here? We use it in the sim now everywhere else. > Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:22 > +CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; This one seems odd. is this strict selector matching? > Source/ThirdParty/ANGLE/Configurations/Version.xcconfig:23 > + Do you need to tell someone this is here so they update it?
(In reply to Tim Horton from comment #16) > Comment on attachment 384551 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=384551&action=review > > > Source/ThirdParty/ANGLE/Configurations/ANGLE.xcconfig:10 > > +ANGLE_OTHER_LDFLAGS_iphonesimulator = -framework OpenGLES; > > Why no IOSurface here? We use it in the sim now everywhere else. Unfortunately the sim is missing the one crucial method we need: [EAGLContext texImageIOSurface:] > > > Source/ThirdParty/ANGLE/Configurations/Base.xcconfig:22 > > +CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; > > This one seems odd. is this strict selector matching? I will remove it. > > > Source/ThirdParty/ANGLE/Configurations/Version.xcconfig:23 > > + > > Do you need to tell someone this is here so they update it? Yeah, I wondered about that - or decided to hope they do it by find&replace.
Committed r252992: <https://trac.webkit.org/changeset/252992>
Committed r253824: <https://trac.webkit.org/changeset/253824>