Bug 221333 - [CMake] Hide libwebrtc symbols
Summary: [CMake] Hide libwebrtc symbols
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Víctor M. Jáquez L.
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-03 09:01 PST by Víctor M. Jáquez L.
Modified: 2021-02-05 07:12 PST (History)
12 users (show)

See Also:

Attachments
Patch (3.31 KB, patch)
2021-02-03 09:01 PST, Víctor M. Jáquez L. 		no flags Details | Formatted Diff | Diff
Patch (1.70 KB, patch)
2021-02-04 06:16 PST, Víctor M. Jáquez L. 		no flags Details | Formatted Diff | Diff
Patch (1.65 KB, patch)
2021-02-04 10:30 PST, Víctor M. Jáquez L. 		no flags Details | Formatted Diff | Diff
Patch (1.60 KB, patch)
2021-02-04 10:37 PST, Víctor M. Jáquez L. 		no flags Details | Formatted Diff | Diff
Patch (1.61 KB, patch)
2021-02-05 00:53 PST, Víctor M. Jáquez L. 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Víctor M. Jáquez L. 2021-02-03 09:01:04 PST 
Hide libwebrtc symbols
Comment 1 Víctor M. Jáquez L. 2021-02-03 09:01:50 PST 
Created attachment 419144 [details]
Patch
Comment 2 Víctor M. Jáquez L. 2021-02-03 09:02:52 PST 
Michael,

Can you confirm if this works in Fedora?
Comment 3 Michael Catanzaro 2021-02-03 15:14:12 PST 
Clever idea... I didn't get to it today, but will try soon.
Comment 4 Michael Catanzaro 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)
Comment 5 Víctor M. Jáquez L. 2021-02-04 06:16:16 PST 
Created attachment 419277 [details]
Patch
Comment 6 Víctor M. Jáquez L. 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 :)
Comment 7 Michael Catanzaro 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.
Comment 8 Michael Catanzaro 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. ;)
Comment 9 Víctor M. Jáquez L. 2021-02-04 10:30:55 PST 
Created attachment 419298 [details]
Patch
Comment 10 Víctor M. Jáquez L. 2021-02-04 10:37:52 PST 
Created attachment 419299 [details]
Patch
Comment 11 Darin Adler 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?
Comment 12 Adrian Perez 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 :)
Comment 13 Víctor M. Jáquez L. 2021-02-05 00:53:29 PST 
Created attachment 419369 [details]
Patch
Comment 14 EWS 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].
Comment 15 Radar WebKit Bug Importer 2021-02-05 07:12:15 PST 
<rdar://problem/74024184>