Skip to content

src/CMakeLists.txt: modernize CMake build system and fix package config#45

Open
luadebug wants to merge 15 commits intowinsoft666:masterfrom
luadebug:patch-1
Open

src/CMakeLists.txt: modernize CMake build system and fix package config#45
luadebug wants to merge 15 commits intowinsoft666:masterfrom
luadebug:patch-1

Conversation

@luadebug
Copy link

@luadebug luadebug commented Dec 23, 2025

  • Use CONFIG mode for find_package(CURL) with imported target CURL::libcurl
  • Simplify dependency linking to always use PRIVATE (transitive deps handled by imported targets)
  • Fix INSTALL_INTERFACE include path from ../include to include
  • Add proper package config file generation using configure_package_config_file
  • Rename export from zoeConfig to zoeTargets for CMake conventions
  • Simplify Windows platform check and library names
  • Remove redundant OpenSSL::Crypto (linked transitively via OpenSSL::SSL)

Add zoeConfig.cmake.in template that:

  • Uses CMakeFindDependencyMacro for transitive dependency resolution
  • Conditionally finds CURL and OpenSSL dependencies for static builds
  • Includes generated zoeTargets.cmake for exported target definitions

This ensures consumers using find_package(zoe) get proper dependency propagation when linking against the static library.

Enhance CMake configuration to support dynamic linking with libcurl and OpenSSL, and add pkg-config file for better integration

- Use CONFIG mode for find_package(CURL) with imported target CURL::libcurl
- Simplify dependency linking to always use PRIVATE (transitive deps handled by imported targets)
- Fix INSTALL_INTERFACE include path from ../include to include
- Add proper package config file generation using configure_package_config_file
- Rename export from zoeConfig to zoeTargets for CMake conventions
- Simplify Windows platform check and library names
- Remove redundant OpenSSL::Crypto (linked transitively via OpenSSL::SSL)
Add zoeConfig.cmake.in template that:
- Uses CMakeFindDependencyMacro for transitive dependency resolution
- Conditionally finds CURL and OpenSSL dependencies for static builds
- Includes generated zoeTargets.cmake for exported target definitions

This ensures consumers using find_package(zoe) get proper dependency
propagation when linking against the static library.
…nd OpenSSL, and add pkg-config file for better integration
@luadebug luadebug force-pushed the patch-1 branch 4 times, most recently from 294f034 to 8db2f97 Compare December 24, 2025 00:05
Copy link

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was about to upstream my fixes from the vcpkg port. But there is overlap with this PR.

if(ZOE_BUILD_SHARED_LIBS)
set(ZOE_PRIVATE_LIBS "${ZOE_PRIVATE_LIBS} -lpthread")
else()
set(ZOE_LIBS "${ZOE_LIBS} -lpthread")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this leads to the wrong result for MSVC.

Copy link
Author

@luadebug luadebug Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry maybe I am missing something?
On MSVC, CMake's find_package(Threads) sets:
CMAKE_USE_WIN32_THREADS_INIT = TRUE
CMAKE_USE_PTHREADS_INIT = FALSE

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, CMAKE_USE_PTHREADS_INIT is a result variable. I tend to read this identifier wrong. So no problem with MSVC.

But IIRC -lpthread is also somewhat legacy vs. -pthread. (This is why CMake recommends THREADS_PREFER_PTHREAD_FLAG.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6f76eac
Is it good to go or there is better way to fix, LMK.

luadebug and others added 4 commits December 27, 2025 12:49
Co-authored-by: Kai Pastor <dg0yt@darc.de>
Co-authored-by: Kai Pastor <dg0yt@darc.de>
@luadebug luadebug requested a review from dg0yt December 27, 2025 10:03
Comment on lines 142 to 143
"${CMAKE_CURRENT_BINARY_DIR}/${ZOE_PC_NAME}.pc"
DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, this is meant to generate different pc file names?
This is not common practice in pkgconfig and should be avoided IMO.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LMK if that is good to go according to f43e62d

luadebug and others added 3 commits December 27, 2025 20:54
Co-authored-by: Kai Pastor <dg0yt@darc.de>
Co-authored-by: Kai Pastor <dg0yt@darc.de>
@luadebug luadebug requested a review from dg0yt December 27, 2025 18:07
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.

2 participants