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

Narrow exports of shared libraries #371

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rpavlik
Copy link
Contributor

@rpavlik rpavlik commented Apr 22, 2022

Builds on #370 .

I'm working on packaging for vcpkg (see microsoft/vcpkg#24310 ) and I noticed that their maintainer guide recommends not allowing Windows shared library builds for libraries with CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS. I thought it would be easy to add export declarations, since there's a clear separate API header. However, it turns out that several examples and tests use nominally "internal" APIs, so in most cases I added additional export defines just to highlight those semi-public APIs. (One usage of a private API is called out in a comment, so that build I just disabled when building for shared libraries).

Curious what you would like to do about those. We could also go to a linker version script/.def file instead of the defines if you prefer. This is mostly a request-for-comment, though I certainly wouldn't mind if it got merged.

rpavlik added 2 commits April 25, 2022 09:12
Does not perform transitive link to boringssl though.
@litespeedtech
Copy link
Owner

Can you please update the pull request to resolve the conflicts. thanks!

@rpavlik
Copy link
Contributor Author

rpavlik commented Apr 28, 2022

Yeah, will do. The one question I do have is about the tests - many of them link against what are probably "implementation details". Do we just want to skip those tests when doing a shared library build? The alternative is manually re-adding the particular source files to each test, which I got some progress on but which is super tedious, or just also building a static version for the tests.

@litespeedtech
Copy link
Owner

Those tests are intended to test the internals, need to access some internal functions. :-)
I think the simplest way is to static link test binaries.

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.

2 participants