Conversation
…g, conditional examples 1. Add --disable-shared and --disable-static options to configure/makefile 2. Make CMake static library output name consistent with Makefile (libhv.a) 3. Make Makefile examples target conditional based on module settings 4. Add multi-thread examples to CMakeLists.txt Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves the configure/Makefile build system for the libhv library to address three issues: lack of --disable-shared/--disable-static support, inconsistent static library naming between CMake and Makefile builds, and unconditional example building regardless of module settings.
Changes:
- Added
BUILD_SHARED/BUILD_STATICoptions toconfig.ini,config.mk,configure, andMakefileto support selectively building shared/static libraries - Set
OUTPUT_NAME hvon the CMakehv_statictarget so both build systems producelibhv.ainstead oflibhv_static.a - Made example targets conditional on module flags (
WITH_EVPP,WITH_HTTP, etc.) in the Makefile, and added missing multi-thread examples toexamples/CMakeLists.txt
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
config.ini |
Added BUILD_SHARED=yes and BUILD_STATIC=yes defaults |
config.mk |
Replaced with full commented template matching config.ini; added BUILD_SHARED/BUILD_STATIC; removed hardcoded CONFIG_DATE |
configure |
Added --enable/disable-shared and --enable/disable-static cases before generic handlers; added help text |
Makefile |
Made libhv target conditionally build shared/static based on flags; made examples target conditional on module flags |
CMakeLists.txt |
Added set_target_properties(hv_static PROPERTIES OUTPUT_NAME hv) for consistent naming |
examples/CMakeLists.txt |
Added missing multi-thread examples (multi-acceptor-processes, multi-acceptor-threads, one-acceptor-multi-workers) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Don't modify this file, you should modify config.mk or | ||
| # run ./configure --with-MODULE --enable-FEATURE |
There was a problem hiding this comment.
The comment "Don't modify this file, you should modify config.mk" is self-referential when placed in config.mk itself. This comment was originally intended for config.ini (the template). In config.mk, the comment should either be removed or changed to something like "# This file is generated by ./configure. Edit config.ini or run ./configure to change settings."
| target_include_directories(hv_static PRIVATE ${LIBHV_SRCDIRS} | ||
| INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include> $<INSTALL_INTERFACE:include>) | ||
| target_link_libraries(hv_static ${LIBS}) | ||
| set_target_properties(hv_static PROPERTIES OUTPUT_NAME hv) |
There was a problem hiding this comment.
On Windows with MSVC, setting OUTPUT_NAME hv on the static target will cause a naming conflict when both BUILD_SHARED and BUILD_STATIC are enabled (the default). The shared library hv already produces an import library hv.lib, and now the static library hv_static will also produce hv.lib in the same output directory, causing one to overwrite the other.
A common solution is to also set ARCHIVE_OUTPUT_NAME to a different name (e.g., hv_static) on the static target only when BUILD_SHARED is also enabled, or alternatively to use the CMAKE_STATIC_LIBRARY_SUFFIX workaround. Another approach is to guard this property with a condition like if(NOT BUILD_SHARED OR NOT WIN32).
|
@copilot apply changes based on the comments in this thread |
…ming conflict 1. Remove self-referential comment from config.mk (comment was for config.ini) 2. Guard OUTPUT_NAME hv on static target to avoid .lib conflict on Windows when both BUILD_SHARED and BUILD_STATIC are enabled Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com>
Applied both review changes in 9f9eaf5:
|
The configure/makefile build system was missing
--disable-shared/--disable-staticsupport, producing differently-named static libraries than CMake (libhv.avslibhv_static.a), and unconditionally building all examples regardless of module settings.--disable-shared/--disable-staticsupportBUILD_SHAREDandBUILD_STATICoptions toconfig.ini/config.mkconfigurebefore the generic--enable-*/--disable-*handlersMakefilelibhvtarget now conditionally setsTARGET_TYPEbased on these flagsConsistent static library naming
set_target_properties(hv_static PROPERTIES OUTPUT_NAME hv)inCMakeLists.txt, guarded byif(NOT (WIN32 AND BUILD_SHARED))to avoid.libnaming conflict on Windows MSVC when both shared and static libraries are builtlibhv.a/libhv.sohv_static.libto avoid overwriting the shared import libraryhv.libConditional examples in Makefile
examplestarget now gates onWITH_EVPP,WITH_HTTP,WITH_HTTP_SERVER,WITH_HTTP_CLIENT,WITH_MQTT— matching the existingexamples/CMakeLists.txtconditionalsmulti-acceptor-processes,multi-acceptor-threads,one-acceptor-multi-workers) toexamples/CMakeLists.txtOriginal prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.