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

Always set the symbol visibility preset to hidden. #1143

Closed
wants to merge 3 commits into from

Conversation

teo-tsirpanis
Copy link
Contributor

Issue #, if available: Fixes #1142

Description of changes:

This PR updates AwsCFlags.cmake to always pass -fvisibility=hidden to the targets, even when we are building static libraries. Instead of directly specifying the compiler option, we use the VISIBILITY_PRESET CMake property, for increased portability across compilers.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@graebm
Copy link
Contributor

graebm commented Aug 19, 2024

Sorry for the silence. I've been chipping away at this

  • I noticed this change caused CMake to spit out a warning about cmake-policy CMP0063.
  • So I made my own branch which did set that policy, but we'd need to update ALL repos (aws-c-http, aws-c-s3, there are more than a dozen others...) to set that policy as well.
  • Which made the team wonder why we didn't just update the minimum CMake version, so we wouldn't need to manually set that policy. Updating CMake is something we've wanted to do for years, so now I'm testing that out in the update-cmake branch

@waahm7
Copy link
Contributor

waahm7 commented Oct 24, 2024

Thank you for the PR, This was fixed in https://github.com/awslabs/aws-c-common/releases/tag/v0.10.0.

@waahm7 waahm7 closed this Oct 24, 2024
@teo-tsirpanis teo-tsirpanis deleted the fvisibility-hidden branch October 24, 2024 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider enabling -fvisibility=hidden when building a static library.
4 participants