RESOLVED FIXED 204708
Build ANGLE as a dynamic library
https://bugs.webkit.org/show_bug.cgi?id=204708
Summary Build ANGLE as a dynamic library
Dean Jackson
Reported 2019-11-29 13:47:35 PST
Build ANGLE as a dynamic library
Attachments
Patch (44.84 KB, patch)
2019-11-29 14:02 PST, Dean Jackson
no flags
Patch (47.64 KB, patch)
2019-11-29 14:25 PST, Dean Jackson
no flags
Patch (49.18 KB, patch)
2019-11-29 16:05 PST, Dean Jackson
no flags
Patch (41.15 KB, patch)
2019-11-29 16:15 PST, Dean Jackson
no flags
Patch (54.09 KB, patch)
2019-11-30 12:14 PST, Dean Jackson
no flags
Patch (54.09 KB, patch)
2019-11-30 12:40 PST, Dean Jackson
no flags
Patch (54.09 KB, patch)
2019-11-30 14:14 PST, Dean Jackson
no flags
Patch (54.26 KB, patch)
2019-11-30 15:03 PST, Dean Jackson
thorton: review+
Dean Jackson
Comment 1 2019-11-29 13:48:46 PST
Dean Jackson
Comment 2 2019-11-29 14:02:50 PST
EWS Watchlist
Comment 3 2019-11-29 14:03:34 PST
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Dean Jackson
Comment 4 2019-11-29 14:25:33 PST
mitz
Comment 5 2019-11-29 15:06:52 PST
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?
Dean Jackson
Comment 6 2019-11-29 16:05:56 PST
Dean Jackson
Comment 7 2019-11-29 16:09:37 PST
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?
Dean Jackson
Comment 8 2019-11-29 16:15:10 PST
mitz
Comment 9 2019-11-29 16:15:52 PST
(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.
Dean Jackson
Comment 10 2019-11-29 16:44:21 PST
(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.
Dean Jackson
Comment 11 2019-11-30 12:14:23 PST
Dean Jackson
Comment 12 2019-11-30 12:40:01 PST
Dean Jackson
Comment 13 2019-11-30 14:14:10 PST
Dean Jackson
Comment 14 2019-11-30 15:03:32 PST
Dean Jackson
Comment 15 2019-11-30 15:37:07 PST
Finally got all the ports building. Also tested with the Apple-internal build system.
Tim Horton
Comment 16 2019-12-02 10:00:15 PST
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?
Dean Jackson
Comment 17 2019-12-02 10:24:18 PST
(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.
Dean Jackson
Comment 18 2019-12-02 10:36:21 PST
Dean Jackson
Comment 19 2019-12-20 09:34:35 PST
Note You need to log in before you can comment on or make changes to this bug.