RESOLVED FIXED 221333
[CMake] Hide libwebrtc symbols 
https://bugs.webkit.org/show_bug.cgi?id=221333
Summary [CMake] Hide libwebrtc symbols
Víctor M. Jáquez L.
Reported 2021-02-03 09:01:04 PST
Hide libwebrtc symbols
Attachments
Patch (3.31 KB, patch)
2021-02-03 09:01 PST, Víctor M. Jáquez L. 		no flags
DetailsFormatted DiffDiff
Patch (1.70 KB, patch)
2021-02-04 06:16 PST, Víctor M. Jáquez L. 		no flags
DetailsFormatted DiffDiff
Patch (1.65 KB, patch)
2021-02-04 10:30 PST, Víctor M. Jáquez L. 		no flags
DetailsFormatted DiffDiff
Patch (1.60 KB, patch)
2021-02-04 10:37 PST, Víctor M. Jáquez L. 		no flags
DetailsFormatted DiffDiff
Patch (1.61 KB, patch)
2021-02-05 00:53 PST, Víctor M. Jáquez L. 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Víctor M. Jáquez L.
Comment 1 2021-02-03 09:01:50 PST
Created attachment 419144 [details] Patch
Víctor M. Jáquez L.
Comment 2 2021-02-03 09:02:52 PST
Michael, Can you confirm if this works in Fedora?
Michael Catanzaro
Comment 3 2021-02-03 15:14:12 PST
Clever idea... I didn't get to it today, but will try soon.
Michael Catanzaro
Comment 4 2021-02-03 15:21:33 PST
Comment on attachment 419144 [details] Patch I wonder if this would work: set_target_properties(webrtc PROPERTIES CXX_VISIBILITY_PRESET hidden) set_target_properties(webrtc PROPERTIES C_VISIBILITY_PRESET hidden)
Víctor M. Jáquez L.
Comment 5 2021-02-04 06:16:16 PST
Created attachment 419277 [details] Patch
Víctor M. Jáquez L.
Comment 6 2021-02-04 06:16:57 PST
(In reply to Michael Catanzaro from comment #4) > Comment on attachment 419144 [details] > Patch > > I wonder if this would work: > > set_target_properties(webrtc PROPERTIES CXX_VISIBILITY_PRESET hidden) > set_target_properties(webrtc PROPERTIES C_VISIBILITY_PRESET hidden) It does :)
Michael Catanzaro
Comment 7 2021-02-04 08:33:01 PST
Comment on attachment 419277 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419277&action=review (Still hoping to test this out soon.) > Source/ThirdParty/libwebrtc/CMakeLists.txt:1429 > +if (UNIX AND COMPILER_IS_GCC_OR_CLANG) This probably isn't needed, right? The point of using CMake's builtin properties is to ensure you don't need to worry about platform-specific details.
Michael Catanzaro
Comment 8 2021-02-04 10:05:19 PST
Comment on attachment 419277 [details] Patch Tested and confirmed fixed. I think it can safely be done unconditionally though since libwebrtc stuff should never be exported. Eventually we should try to properly fix bug #181916. In the meantime... this was good thinking. ;)
Víctor M. Jáquez L.
Comment 9 2021-02-04 10:30:55 PST
Created attachment 419298 [details] Patch
Víctor M. Jáquez L.
Comment 10 2021-02-04 10:37:52 PST
Created attachment 419299 [details] Patch
Darin Adler
Comment 11 2021-02-04 12:12:03 PST
This CMake-only fix has a title wording that makes it sound like it works for all platforms and build systems. Maybe add a [CMake] prefix?
Adrian Perez
Comment 12 2021-02-04 12:26:09 PST
(In reply to Darin Adler from comment #11) > This CMake-only fix has a title wording that makes it sound like it works > for all platforms and build systems. Maybe add a [CMake] prefix? Yes, I have added the tag to the bug title. Victor, could you please update the ChangeLog and reupload? Even when we should aim to build everything with hidden visibility as the default (bug #181916), this is a no-brainer stopgap measure for libwebrtc and I think we should land this patch :)
Víctor M. Jáquez L.
Comment 13 2021-02-05 00:53:29 PST
Created attachment 419369 [details] Patch
EWS
Comment 14 2021-02-05 07:11:49 PST
Committed r272413: <https://trac.webkit.org/changeset/272413> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419369 [details].
Radar WebKit Bug Importer
Comment 15 2021-02-05 07:12:15 PST
<rdar://problem/74024184>
Note You need to log in before you can comment on or make changes to this bug.