-
Notifications
You must be signed in to change notification settings - Fork 0
Add BUILD_BOTH_LIBRARIES option for TAP tests #2
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
Changes from all commits
9d3f65b
5222c99
7bed243
9cf61b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,36 +12,92 @@ else() | |||||||||||||||||||||||||||||||||||||||||||||||||||
| endif() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| message(STATUS "Building CPP-DOTENV in ${CMAKE_BUILD_TYPE} mode") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Option to build both static and shared libraries | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| option(BUILD_BOTH_LIBRARIES "Build both static and shared libraries" OFF) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Validate that BUILD_BOTH_LIBRARIES and BUILD_SHARED_LIBS are not used together | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+15
to
+18
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Option to build both static and shared libraries | |
| option(BUILD_BOTH_LIBRARIES "Build both static and shared libraries" OFF) | |
| # Validate that BUILD_BOTH_LIBRARIES and BUILD_SHARED_LIBS are not used together | |
| # Option to build both static and shared libraries. | |
| # Usage: | |
| # - Default behavior (BUILD_BOTH_LIBRARIES=OFF): | |
| # * Only one library target named `cpp_dotenv` is built. | |
| # * Its type (STATIC or SHARED) is controlled by CMake's standard | |
| # `BUILD_SHARED_LIBS` option. | |
| # - BUILD_BOTH_LIBRARIES=ON: | |
| # * Two libraries are built: | |
| # - `cpp_dotenv_static` (STATIC) | |
| # - `cpp_dotenv_shared` (SHARED) | |
| # * The `cpp_dotenv` target becomes an ALIAS to `cpp_dotenv_static` | |
| # for backward compatibility. | |
| # * Projects that need the shared library must explicitly link against | |
| # `cpp_dotenv_shared`. | |
| # - Example CMake invocation: | |
| # cmake -DBUILD_BOTH_LIBRARIES=ON .. | |
| option(BUILD_BOTH_LIBRARIES "Build both static and shared libraries" OFF) | |
| # Validate that BUILD_BOTH_LIBRARIES and BUILD_SHARED_LIBS are not used together. | |
| # When both are ON, BUILD_SHARED_LIBS is ignored and forced OFF to ensure a | |
| # predictable configuration. |
Copilot
AI
Jan 21, 2026
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.
The validation logic forces BUILD_SHARED_LIBS to OFF when BUILD_BOTH_LIBRARIES is enabled, but this might not work as intended in all scenarios. When BUILD_SHARED_LIBS is set via command line with -DBUILD_SHARED_LIBS=ON, the FORCE option may cause the cache to be updated, but the order of evaluation could still cause issues. Consider using a FATAL_ERROR instead of WARNING and forcing the value, which would require users to explicitly fix their configuration rather than silently changing behavior.
| message(WARNING "BUILD_BOTH_LIBRARIES and BUILD_SHARED_LIBS should not be used together. BUILD_SHARED_LIBS will be ignored.") | |
| set(BUILD_SHARED_LIBS OFF CACHE BOOL "Build shared libraries" FORCE) | |
| endif() | |
| message(FATAL_ERROR "BUILD_BOTH_LIBRARIES and BUILD_SHARED_LIBS must not be enabled at the same time. Please disable one of them and re-configure.") | |
| endif() |
Copilot
AI
Jan 21, 2026
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.
When BUILD_BOTH_LIBRARIES is ON, the shared library cpp_dotenv_shared will link against static libraries (ENVIRON_LIB and PARSER_LIB). For this to work correctly on most platforms, these static libraries need to be compiled with position-independent code (-fPIC). Consider setting CMAKE_POSITION_INDEPENDENT_CODE to ON globally or for the dependent libraries when BUILD_BOTH_LIBRARIES is enabled to ensure the shared library builds correctly.
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.
🧩 Analysis chain
🏁 Script executed:
# Find CMakeLists.txt and read the full context around the problematic lines
find . -name "CMakeLists.txt" -type fRepository: ProxySQL/cpp-dotenv
Length of output: 575
🏁 Script executed:
# Read the CMakeLists.txt file to see the full context
# Let's first check its size
wc -l CMakeLists.txtRepository: ProxySQL/cpp-dotenv
Length of output: 80
🏁 Script executed:
# Read the relevant sections of CMakeLists.txt
cat -n CMakeLists.txt | head -100Repository: ProxySQL/cpp-dotenv
Length of output: 4262
Prevent Windows .lib import library collision when building both variants.
With OUTPUT_NAME set to cpp_dotenv for both targets, MSVC on Windows generates a .lib file for the static library and an import library with the same name for the shared DLL, causing an overwrite. On Unix/Linux (.a/.so) and macOS (.a/.dylib), there is no collision. If the BUILD_BOTH_LIBRARIES option is used on Windows, the shared target's import library must have a distinct name or output directory.
🛠️ Suggested fix (keep DLL name, rename import .lib)
# Set output name to libcpp_dotenv.so/.dylib (instead of libcpp_dotenv_shared.so)
set_target_properties(cpp_dotenv_shared PROPERTIES OUTPUT_NAME cpp_dotenv)
+ if(WIN32)
+ # Avoid collision with the static cpp_dotenv.lib
+ set_target_properties(cpp_dotenv_shared PROPERTIES ARCHIVE_OUTPUT_NAME cpp_dotenv_shared)
+ endif()🤖 Prompt for AI Agents
In `@CMakeLists.txt` around lines 65 - 74, The shared target cpp_dotenv_shared
currently sets OUTPUT_NAME to cpp_dotenv which conflicts with the static lib
import on MSVC; change the shared target properties to keep the DLL name as
cpp_dotenv but give the import library a distinct name (use
set_target_properties(cpp_dotenv_shared PROPERTIES OUTPUT_NAME cpp_dotenv
IMPORT_LIBRARY cpp_dotenv_shared_import) or similar) so when
BUILD_BOTH_LIBRARIES is enabled on Windows the .lib import file won’t overwrite
the static .lib; apply this to the cpp_dotenv_shared target (modify the
set_target_properties call) and conditionally do it only for MSVC/Windows if you
prefer.
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.
The BUILD_BOTH_LIBRARIES option should validate that it's not used together with BUILD_SHARED_LIBS=ON, or add a warning message. Add validation after line 16:
if(BUILD_BOTH_LIBRARIES AND BUILD_SHARED_LIBS)
message(WARNING "BUILD_BOTH_LIBRARIES and BUILD_SHARED_LIBS should not be used together. BUILD_SHARED_LIBS will be ignored.")
set(BUILD_SHARED_LIBS OFF)
endif()
This prevents the problematic scenario where static libraries would link against shared dependencies.