WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(47.64 KB, patch)
2019-11-29 14:25 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Patch
(49.18 KB, patch)
2019-11-29 16:05 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Patch
(41.15 KB, patch)
2019-11-29 16:15 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Patch
(54.09 KB, patch)
2019-11-30 12:14 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Patch
(54.09 KB, patch)
2019-11-30 12:40 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Patch
(54.09 KB, patch)
2019-11-30 14:14 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Patch
(54.26 KB, patch)
2019-11-30 15:03 PST
,
Dean Jackson
thorton
: review+
Details
Formatted Diff
Diff
Show Obsolete
(7)
View All
Add attachment
proposed patch, testcase, etc.
Dean Jackson
Comment 1
2019-11-29 13:48:46 PST
rdar://57349384
Dean Jackson
Comment 2
2019-11-29 14:02:50 PST
Created
attachment 384524
[details]
Patch
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
Created
attachment 384527
[details]
Patch
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
Created
attachment 384530
[details]
Patch
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
Created
attachment 384531
[details]
Patch
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
Created
attachment 384546
[details]
Patch
Dean Jackson
Comment 12
2019-11-30 12:40:01 PST
Created
attachment 384547
[details]
Patch
Dean Jackson
Comment 13
2019-11-30 14:14:10 PST
Created
attachment 384548
[details]
Patch
Dean Jackson
Comment 14
2019-11-30 15:03:32 PST
Created
attachment 384551
[details]
Patch
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
Committed
r252992
: <
https://trac.webkit.org/changeset/252992
>
Dean Jackson
Comment 19
2019-12-20 09:34:35 PST
Committed
r253824
: <
https://trac.webkit.org/changeset/253824
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug