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

Make crypto static library targets explicitly STATIC #919

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

jagerman
Copy link
Contributor

@jagerman jagerman commented Aug 29, 2023

Not having the STATIC keyword in add_library(...) makes CMake go look for a BUILD_SHARED_LIBS variable and, if set, makes the "static" library crypto targets actually create a shared library.

This causes a problem if including ngtcp2 as a subproject from some other project that itself happens to have a default of BUILD_SHARED_LIBS=ON, resulting in the "static" targets getting built dynamically.

This fixes it by making the crypto static library targets explicitly STATIC so that this cmake messiness doesn't happen (and also matches the way the main ngtcp2_static target is created).

Not having the STATIC keyword in `add_library(...)` makes CMake go look
for a `BUILD_SHARED_LIBS` variable and, if set, makes the "static"
library crypto targets actually create a shared library.

This causes a problem if including ngtcp2 as a subproject from some
other project that itself happens to have a default of true, resulting
in the "static" targets getting built dynamically.

This fixes it by making the crypto static library targets explicitly
STATIC so that this cmake messiness doesn't happen (and also matches the
way the main ngtcp2_static target is created).
@jagerman jagerman changed the title Make crypto libraries explicitly STATIC Make crypto static library targets explicitly STATIC Aug 29, 2023
@tatsuhiro-t tatsuhiro-t added this to the v0.19.0 milestone Aug 30, 2023
@tatsuhiro-t tatsuhiro-t merged commit c92c8af into ngtcp2:main Aug 30, 2023
22 checks passed
@tatsuhiro-t
Copy link
Member

Thank you for PR. Merged now.

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.

None yet

2 participants