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

Add CMAKE options to be able to build shared and static versions of the library at the same time #57

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Peter2121
Copy link

I want to create a FreeBSD port of this library (as I need it for one project). The FreeBSD ports system does no permit to define build options for a parent port (dependency) at the level of a main port that uses the dependency. So, anyone who want to use the library needs to manually build the port if he wants to use a static version.

This commit permits to build both shared and static versions of the library (only shared version is built by default - like in the original version of build file).

The issue for this problem: #56

I fixed the Wiki, but I don't know how to create a PR for it.

@Amanieu
Copy link
Owner

Amanieu commented Feb 11, 2024

I haven't touched cmake in ~10 years, so I don't feel qualified to review this. I'm happy to merge it if someone else can chime in and say this looks OK.

@2xsaiko
Copy link

2xsaiko commented Jan 9, 2025

Please don't do this. A single CMake build directory should build either the static or shared library, controlled via BUILD_SHARED_LIBS as is right now.

The way to install both is something like this instead, which still respects BUILD_SHARED_LIBS: https://alexreinking.com/blog/building-a-dual-shared-and-static-library-with-cmake.html

@Peter2121
Copy link
Author

A single CMake build directory should build either the static or shared library

Could you, please justify this point? Is it your personal opinion, the best practice mentioned elsewhere or a technical requirement of something?
The patch provided permits to build both libraries during one build. It simplifies the package management. Why should not we do it?

@2xsaiko
Copy link

2xsaiko commented Jan 9, 2025

Could you, please justify this point? Is it your personal opinion, the best practice mentioned elsewhere or a technical requirement of something?

  1. It should be possible to link against a CMake target by just its name (i.e. "target_link_libraries(Foo Async++)") in both the in-project and install interface, regardless of whether it is a shared or static library.
  2. Using BUILD_SHARED_LIBS along with the method in the article I've linked makes selecting which variant to use controllable by the user (packager, or just whoever builds it, or a project that uses it)
  3. It also means developers using this project don't have to hardcode which library to use (your async++ vs async++-static)
  4. CMake is intended to work that way, see builtin variables BUILD_SHARED_LIBS and add_library() without specifying library type

Additionally you're duplicating A LOT of code here, even if two targets was the way to go I would never merge this on that basis alone.

Read the article I've linked.

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.

3 participants