Bug 221333

Summary:

[CMake] Hide libwebrtc symbols

Product:

WebKit

Reporter:

Víctor M. Jáquez L. <vjaquez>

Component:

WebRTC

Assignee:

Víctor M. Jáquez L. <vjaquez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

annulen, aperez, darin, don.olmstead, eric.carlson, ews-watchlist, gyuyoung.kim, mcatanzaro, ryuan.choi, sergio, webkit-bug-importer, youennf

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=181916
https://bugs.webkit.org/show_bug.cgi?id=214812

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

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	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 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.