Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Library symbols should be hidden by default #166

Closed
azeey opened this issue Apr 28, 2021 · 6 comments · Fixed by #196
Closed

Library symbols should be hidden by default #166

azeey opened this issue Apr 28, 2021 · 6 comments · Fixed by #196
Labels
🔼 gz-cmake4 Changes waiting a new major bump bug Something isn't working

Comments

@azeey
Copy link
Contributor

azeey commented Apr 28, 2021

Environment

  • OS Version: Ubuntu 18.04
  • Source or binary build? source, ign-cmake2 (17b7058)

Description

Steps to reproduce

Here's an MWE: https://github.com/azeey/sandbox/tree/master/ign-cmake-visibility

After building, use nm -C lib/libtset_ign_cmake_visibility.so | grep test_ to see which symbols are hidden:

 nm -C lib/libtest_ign_cmake_visibility.so | grep test_
00000000000005c0 T test_hidden_func(int, int)
00000000000005e0 T test_visible_func(int, int)
00000000000005d0 t test_explicitly_hidden_func(int, int)

test_hidden_func should be hidden by default, but as the T (global symbol) in the nm output shows, it is visibile.

@azeey azeey added the bug Something isn't working label Apr 28, 2021
@chapulina
Copy link
Contributor

Something to target ign-cmake3. We could at least update the docs now.

@scpeters
Copy link
Member

keep open until we change the default behavior in ignition-cmake3

@j-rivero j-rivero added the 🔼 gz-cmake4 Changes waiting a new major bump label Apr 7, 2022
@scpeters
Copy link
Member

I guess we forgot to reconfigure the default behavior in gz-cmake3

@j-rivero
Copy link
Contributor

I guess we forgot to reconfigure the default behavior in gz-cmake3

Unfortunately we lost the moment to get the changes done. I've renamed the label to gz-cmake4. I've a testing workspace ready for testing but last time it was broken on gz-math7 with was quite early in the dependency chain. We probably want to fix the issues before committing the change to gz-cmake to avoid broken compilations on gz family.

@j-rivero
Copy link
Contributor

j-rivero commented Nov 6, 2023

As suggested by @azeey and @scpeters in gazebosim/gz-math#566 (comment). Best way to test CI could be:

  • Pass HIDE_SYMBOLS_BY_DEFAULT to the gz_configure_build calls in every gz-* package
  • Test and verify that CI / nightly builds are happy
  • Remove HIDE_SYMBOLS_BY_DEFAULT from the gz_configure_build calls in every gz-* package
  • Merge Use visibility hidden by default #392

@j-rivero
Copy link
Contributor

As suggested by @azeey and @scpeters in gazebosim/gz-math#566 (comment). Best way to test CI could be:

* Pass HIDE_SYMBOLS_BY_DEFAULT to the gz_configure_build calls in every gz-* package

* Test and verify that CI / nightly builds are happy

All until here is now done.

* Remove HIDE_SYMBOLS_BY_DEFAULT from the gz_configure_build calls in every gz-*    * Merge

As pointed by @scpeters we are here. Let's remove the added symbols from all libs.

j-rivero added a commit to gazebosim/gz-transport that referenced this issue Jan 11, 2024
j-rivero added a commit to gazebosim/gz-physics that referenced this issue Jan 11, 2024
j-rivero added a commit to gazebosim/gz-rendering that referenced this issue Jan 11, 2024
j-rivero added a commit to gazebosim/gz-utils that referenced this issue Jan 11, 2024
j-rivero added a commit to gazebosim/gz-sensors that referenced this issue Jan 11, 2024
j-rivero added a commit to gazebosim/sdformat that referenced this issue Jan 11, 2024
j-rivero added a commit to gazebosim/gz-plugin that referenced this issue Jan 11, 2024
scpeters pushed a commit to gazebosim/gz-physics that referenced this issue Jan 11, 2024
scpeters pushed a commit to gazebosim/gz-math that referenced this issue Jan 11, 2024
mjcarroll pushed a commit to gazebosim/gz-common that referenced this issue Jan 12, 2024
mjcarroll pushed a commit to gazebosim/gz-sim that referenced this issue Jan 12, 2024
mjcarroll pushed a commit to gazebosim/gz-gui that referenced this issue Jan 12, 2024
mjcarroll pushed a commit to gazebosim/gz-msgs that referenced this issue Jan 12, 2024
mjcarroll pushed a commit to gazebosim/gz-rendering that referenced this issue Jan 12, 2024
mjcarroll pushed a commit to gazebosim/gz-transport that referenced this issue Jan 12, 2024
mjcarroll pushed a commit to gazebosim/gz-sensors that referenced this issue Jan 12, 2024
j-rivero added a commit to gazebosim/gz-plugin that referenced this issue Jan 12, 2024
azeey pushed a commit to gazebosim/gz-utils that referenced this issue Jan 18, 2024
azeey pushed a commit to gazebosim/sdformat that referenced this issue Jan 18, 2024
azeey pushed a commit to gazebosim/gz-fuel-tools that referenced this issue Jan 19, 2024
aagrawal05 pushed a commit to aagrawal05/sdformat that referenced this issue Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔼 gz-cmake4 Changes waiting a new major bump bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants