Hi!
This would be very helpful, due to several reasons. But it also carries
several disadvantages. Everything I've found is explained below:
- Currently we need to create and configure two separate targets for the
serf library (serf_static and serf_shared). One for shared build and one
for static. This could be hard to maintain, since stuff in relation to a
new functionality has to be added to both targets and tested separately.
- This would be less confusing to produce and install this one main target.
- CMake has designed a pretty useful framework that handles different build
types. If add_library() was called without directly specifying shared or
static, cmake will automatically determine it based on
BUILD_SHARED_LIBRARIES option.
- It takes twice as much time to build a second instance of the same
library, one of which you wouldn't actually use (in most scenarios).
- However, the tests are currently made so they will compile only against
the static version.
- On the other hand, this means that you can't actually test the shared
version -- the one that you'll most probably use. So it would be better to
fix the tests with shared libs as well. It seems pretty easy to do, since
the only limit is a few hidden private symbols.
That's it... Maybe I missed some points...
A (draft) patch is attached.
--
Timofei Zhakov
Index: CMakeLists.txt
===================================================================
--- CMakeLists.txt (revision 1926089)
+++ CMakeLists.txt (working copy)
@@ -60,8 +60,7 @@
# Build options
option(DEBUG "Enable debugging info and strict compile warnings" OFF)
option(DOT_CLANGD "Generate a .clangd file at the root of the source tree" OFF)
-option(SKIP_SHARED "Disable building shared Serf libraries" OFF)
-option(SKIP_STATIC "Disable building static Serf libraries" OFF)
+option(BUILD_SHARED_LIBS "Build Serf shared libraries" ON)
option(DISABLE_LOGGING "Disable the logging framework at compile time" OFF)
option(SKIP_TESTS "Disable building the unit tests and utilities" OFF)
option(ENABLE_SLOW_TESTS "Enable long-running unit tests" OFF)
@@ -73,10 +72,6 @@
option(USE_HOMEBREW "macOS: Use dependencies from Homebrew" OFF)
option(USE_MACPORTS "macOS: Use dependencies from MacPorts" OFF)
-if(SKIP_SHARED AND SKIP_STATIC)
- message(FATAL_ERROR "You have disabled both shared and static library
builds.")
-endif()
-
if(USE_HOMEBREW AND USE_MACPORTS)
message(FATAL_ERROR "You requested both Homebrew and McPorts dependencies.")
endif()
@@ -187,21 +182,23 @@
)
if(SERF_WINDOWS)
- # Generate the .def file for the Windows DLL import library.
- set(SERF_DEF_FILE "${CMAKE_CURRENT_BINARY_DIR}/serf.def")
- add_custom_command(
- OUTPUT "${SERF_DEF_FILE}"
- DEPENDS ${HEADERS}
- COMMAND ${CMAKE_COMMAND}
- -DCMAKE_SYSTEM_NAME="${CMAKE_SYSTEM_NAME}"
- -DCMAKE_MODULE_PATH="${CMAKE_MODULE_PATH}"
- -DSERF_DEF_BLACKLIST="${EXPORTS_BLACKLIST}"
- -DSERF_DEF_HEADERS="${HEADERS}"
- -DSERF_DEF_FILE="${SERF_DEF_FILE}"
- -P "build/SerfWindowsGenDef.cmake"
- WORKING_DIRECTORY "${SERF_SOURCE_DIR}"
- )
- set(SHARED_SOURCES "serf.rc" "${SERF_DEF_FILE}")
+ if(BUILD_SHARED_LIBS)
+ # Generate the .def file for the Windows DLL import library.
+ set(SERF_DEF_FILE "${CMAKE_CURRENT_BINARY_DIR}/serf.def")
+ add_custom_command(
+ OUTPUT "${SERF_DEF_FILE}"
+ DEPENDS ${HEADERS}
+ COMMAND ${CMAKE_COMMAND}
+ -DCMAKE_SYSTEM_NAME="${CMAKE_SYSTEM_NAME}"
+ -DCMAKE_MODULE_PATH="${CMAKE_MODULE_PATH}"
+ -DSERF_DEF_BLACKLIST="${EXPORTS_BLACKLIST}"
+ -DSERF_DEF_HEADERS="${HEADERS}"
+ -DSERF_DEF_FILE="${SERF_DEF_FILE}"
+ -P "build/SerfWindowsGenDef.cmake"
+ WORKING_DIRECTORY "${SERF_SOURCE_DIR}"
+ )
+ list(APPEND SOURCES "serf.rc" "${SERF_DEF_FILE}")
+ endif()
# Static OpenSSL, APR and APR-Util need additional libraries that are not
# linked by default by CMake. These will be ignored by the linker if they're
@@ -363,48 +360,31 @@
set(CMAKE_SHARED_LIBRARY_PREFIX "${SERF_INSTALL_LIBRARIES}")
endif(NOT MSVC)
-# Define all targets
-if(NOT SKIP_SHARED)
- add_library(serf_shared SHARED ${SOURCES} ${SHARED_SOURCES})
- target_compile_options(serf_shared PUBLIC ${APR_CFLAGS})
- target_include_directories(serf_shared PUBLIC ${SERF_SOURCE_DIR})
- target_link_libraries(serf_shared
- PRIVATE ${SERF_PRIVATE_TARGETS}
- ${SERF_STANDARD_LIBRARIES}
- PUBLIC ${SERF_PUBLIC_TARGETS})
- set_target_properties(serf_shared PROPERTIES
- VERSION ${SERF_VERSION}
- SOVERSION ${SERF_SOVERSION})
- if(SERF_DARWIN AND NOT RELATIVE_RPATH)
- set_target_properties(serf_shared PROPERTIES
- INSTALL_NAME_DIR "${CMAKE_INSTALL_PREFIX}/${SERF_INSTALL_LIBRARIES}")
- endif()
- set(SERF_TARGETS "serf_shared")
+add_library(serf ${SOURCES})
+target_compile_options(serf PUBLIC ${APR_CFLAGS})
+target_include_directories(serf PUBLIC ${SERF_SOURCE_DIR})
+target_link_libraries(serf
+ PRIVATE ${SERF_PRIVATE_TARGETS}
+ ${SERF_STANDARD_LIBRARIES}
+ PUBLIC ${SERF_PUBLIC_TARGETS})
- if(SERF_WINDOWS)
- string(TOLOWER "${CMAKE_BUILD_TYPE}" config)
- if(NOT "${config}" STREQUAL "release")
- install(FILES $<TARGET_PDB_FILE:serf_shared>
- DESTINATION "${SERF_INSTALL_RUNTIME}")
- endif()
- endif()
+set_target_properties(serf
+ PROPERTIES
+ OUTPUT_NAME "serf-${SERF_MAJOR_VERSION}")
+
+if(SERF_DARWIN AND NOT RELATIVE_RPATH)
+ set_target_properties(serf PROPERTIES
+ INSTALL_NAME_DIR "${CMAKE_INSTALL_PREFIX}/${SERF_INSTALL_LIBRARIES}")
endif()
-if(NOT SKIP_STATIC)
- add_library(serf_static STATIC ${SOURCES})
- target_compile_options(serf_static PUBLIC ${APR_CFLAGS})
- target_include_directories(serf_static PUBLIC ${SERF_SOURCE_DIR})
- target_link_libraries(serf_static
- ${SERF_PRIVATE_TARGETS}
- ${SERF_PUBLIC_TARGETS}
- ${SERF_STANDARD_LIBRARIES})
- list(APPEND SERF_TARGETS "serf_static")
+if(SERF_WINDOWS AND BUILD_SHARED_LIBS)
+ string(TOLOWER "${CMAKE_BUILD_TYPE}" config)
+ if(NOT "${config}" STREQUAL "release")
+ install(FILES $<TARGET_PDB_FILE:serf>
+ DESTINATION "${SERF_INSTALL_RUNTIME}")
+ endif()
endif()
-set_target_properties(${SERF_TARGETS}
- PROPERTIES
- OUTPUT_NAME "serf-${SERF_MAJOR_VERSION}")
-
# Generate the .clangd file
if(DOT_CLANGD)
include(SerfGenClangd)
@@ -411,7 +391,7 @@
endif()
# Install targets
-install(TARGETS ${SERF_TARGETS}
+install(TARGETS serf
ARCHIVE DESTINATION "${SERF_INSTALL_LIBS}"
LIBRARY DESTINATION "${SERF_INSTALL_LIBS}"
RUNTIME DESTINATION "${SERF_INSTALL_RUNTIME}")
@@ -451,10 +431,10 @@
if(NOT SKIP_TESTS)
- if(SKIP_STATIC)
+ if(BUILD_SHARED_LIBS)
message(WARNING "The tests depend on the Serf static library")
- message(STATUS "Skipping tests; to silence this message, either remove")
- message(STATUS "the SKIP_STATIC option or add the SKIP_TESTS option.")
+ message(STATUS "Skipping tests; to silence this message, either disable")
+ message(STATUS "BUILD_SHARED_LIBS option or add the SKIP_TESTS option.")
else()
enable_testing()
add_subdirectory(test)
@@ -471,12 +451,6 @@
set(_have_gssapi OFF)
set(_have_sspi OFF)
-if(NOT SKIP_SHARED)
- set(_build_shared ON)
-endif()
-if(NOT SKIP_STATIC)
- set(_build_static ON)
-endif()
if(NOT SKIP_TESTS)
set(_build_tests ON)
endif()
@@ -502,8 +476,7 @@
message(STATUS " target: ................. : ${SERF_TARGET}")
message(STATUS " generator: .............. : ${CMAKE_GENERATOR}")
message(STATUS " build type .............. : ${CMAKE_BUILD_TYPE}")
-message(STATUS " shared libraries .... ... : ${_build_shared}")
-message(STATUS " static libraries ........ : ${_build_static}")
+message(STATUS " shared libraries ........ : ${BUILD_SHARED_LIBS}")
message(STATUS " tests ................... : ${_build_tests}")
message(STATUS " generate .clangd ........ : ${_gen_dot_clangd}")
message(STATUS " Options:")
Index: test/CMakeLists.txt
===================================================================
--- test/CMakeLists.txt (revision 1926089)
+++ test/CMakeLists.txt (working copy)
@@ -50,15 +50,15 @@
foreach(TEST_TARGET ${SIMPLE_TEST_TARGETS})
add_executable(${TEST_TARGET} "${TEST_TARGET}.c")
- add_dependencies(${TEST_TARGET} serf_static)
- target_link_libraries(${TEST_TARGET} serf_static)
+ add_dependencies(${TEST_TARGET} serf)
+ target_link_libraries(${TEST_TARGET} serf)
endforeach()
add_executable(test_all ${TEST_ALL_SOURCES})
-add_dependencies(test_all serf_static)
+add_dependencies(test_all serf)
target_compile_definitions(test_all PRIVATE "-DMOCKHTTP_OPENSSL")
target_include_directories(test_all SYSTEM BEFORE PRIVATE
${SERF_DEPENDENCY_INCLUDES})
-target_link_libraries(test_all serf_static)
+target_link_libraries(test_all serf)
file(GLOB RESPONSE_TEST_CASES
"${CMAKE_CURRENT_SOURCE_DIR}/testcases/*.response")
foreach(TEST_CASE ${RESPONSE_TEST_CASES})
