-
-
Notifications
You must be signed in to change notification settings - Fork 85
Adding a basic CMakeLists for building a static library #254
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
base: master
Are you sure you want to change the base?
Conversation
103f388 to
5ebba79
Compare
|
Please add |
|
Asking for the sake of clarity... What's the overall desired goal for CMake support. e.g. is this about using it via add_subdirectory from another CMake project? |
|
In my use case (Transmission), yes:
https://github.com/transmission/transmission/blob/b372f7b193a2c02864af33093295d26b7caacfb7/CMakeLists.txt#L554-L555 |
CMakeLists.txt
Outdated
| ) | ||
|
|
||
| install(TARGETS ${PROJECT_NAME} DESTINATION lib) | ||
| install(FILES "${CMAKE_BINARY_DIR}/libpsl.h" DESTINATION include) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you stop Transmission from installing the static library and header file of its bundled dependency and possibly conflicting with a separately installed libpsl system package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to prevent the installation; on the contrary, it's very much expected to happen for file to end up in a predictable location. The library is used via ExternalProject module though, so the installation is not happening as part of Transmission installation, but as a step of an isolated build with install prefix pointing somewhere inside the build tree: https://github.com/transmission/transmission/blob/0f7f460c5589c5511c18140c321df5f85c0b7233/cmake/TrMacros.cmake#L201
That said, if we were to include libpsl as a subproject then definitly having an option like e.g. LIBPSL_ENABLE_INSTALL (defaulting to OFF for subproject builds) would be useful. See https://github.com/transmission/libb64/blob/post-2.0.0-transmission/CMakeLists.txt for an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying. For ExternalProject it makes sense to automatically install the results -- less so for add_subdirectory (or FetchContent).
The distinguishing factor is that ExternalProject doesn't actually expect to install libpsl as part of Transmission, it is just doing so into a custom temporary directory as part of recursive (multistage) builds.
(e.g. this approach is also generally incompatible with defining C macros whose value relates to CMAKE_INSTALL_PREFIX)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, here is what I came up with:
40d0f64
ec9f357 to
88685af
Compare
88685af to
40d0f64
Compare
Based on @ckerr code at https://github.com/transmission/libpsl/blob/692c69d2415e1b20378030c08607935a54434087/CMakeLists.txt
Adapted for upstream.
Added a cross-platforms workflow to trigger it on each pull request (ubuntu, windows, macos).