Skip to content

Conversation

@bill18
Copy link
Contributor

@bill18 bill18 commented Dec 27, 2025

To build examples except those could not be built. The following examples failed to build, so I filtered it out in the CMakeLists.txt:
3-Conio
37-WSS-Client
38-WSS
38-WSS-Server
39-WS-load-test
57-mutex

I'm not familiar with the codebase, I don't know how to fix these issues.

To build examples except those could not be built
@EDBCREPO EDBCREPO force-pushed the main branch 2 times, most recently from cae8ddf to 5aefcee Compare December 28, 2025 02:03
@EDBCREPO
Copy link
Member

EDBCREPO commented Dec 28, 2025

Thanks for the PR! However, I can't merge these changes to CMakeLists.txt for a few critical reasons:

  • Broken Transitivity: You removed the target_link_libraries(nodepp INTERFACE ...) block. This is the most important part of the file. This CMake file was designed for integration, not for local compilation. It allows Nodepp to be imported as an external dependency ( via FetchContent or add_subdirectory ).

https://github.com/NodeppOfficial/nodepp?tab=readme-ov-file#dependencies--cmake-integration

  • Example Overload: We don't need to compile every example during the main build process. We use GitHub Actions to verify compilation across Linux, Windows, and Mac for every commit. This keeps the developer's local build fast while maintaining 100% test coverage.

https://github.com/NodeppOfficial/nodepp/actions/runs/20547331566/job/59019675562

But, I like this section:


find_package(Threads REQUIRED)
find_package(OpenSSL REQUIRED)
find_package(ZLIB REQUIRED)

[...]

target_link_libraries(nodepp INTERFACE 
 Threads::Threads
 OpenSSL::SSL
 OpenSSL::Crypto
 ZLIB::ZLIB
$<$<OR:$<BOOL:${MSVC}>,$<STREQUAL:${CMAKE_SYSTEM_NAME},Windows>>:ws2_32> )

@EDBCREPO EDBCREPO closed this Dec 28, 2025
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