Skip to content

Commit

Permalink
cmake: fix ENABLE_DEBUG=ON builds in default/release mode
Browse files Browse the repository at this point in the history
`ENABLE_DEBUG=ON` enables curl-level debug features. Building and
running the curl test suite requires setting this option.

Before this patch, while building the `testdeps` target with debug
features on with the default ('Release') CMake config, the build stopped
with this linker error:
```
ld: CMakeFiles/unit1395.dir/unit1395.c.o: in function `test':
unit1395.c:(.text+0x1a0): undefined reference to `dedotdotify'
A failure has been detected in another branch of the parallel make
```
Ref: https://github.com/curl/curl/actions/runs/9037287098/job/24835990826#step:3:2483

It happened because `dedotdotify` compiled as a static function into
`libcurlu` due to the undefined `DEBUGBUILD` macro. Then `unit1395`
failed to link it.

Even though the build requires the `DEBUGBUILD` macro, our CMake logic
defined it when building with CMake's 'Debug' configuration only. That
configuration is not required to build or run the test suite, nor to use
curl-level debug features.

This patch fixes this by always defining `DEBUGBUILD` when setting
`ENABLE_DEBUG=ON`. Decoupling this custom option from the selected CMake
Release/Debug configuration.

This change may also allow dropping Debug mode in AppVeyor CMake builds,
possibly making them build faster and run smoother.

Also delete the internal variable `ENABLE_CURLDEBUG` which was a copy of
`ENABLE_DEBUG`. Use the latter everywhere instead.

Ref: curl#13583
Closes #xxxxx
  • Loading branch information
vszakats committed May 12, 2024
1 parent 5b9955e commit 130ddfa
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 11 deletions.
2 changes: 1 addition & 1 deletion CMake/CurlSymbolHiding.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ include(CheckCSourceCompiles)
option(CURL_HIDDEN_SYMBOLS "Set to ON to hide libcurl internal symbols (=hide all symbols that aren't officially external)." ON)
mark_as_advanced(CURL_HIDDEN_SYMBOLS)

if(WIN32 AND ENABLE_CURLDEBUG)
if(WIN32 AND ENABLE_DEBUG)
# We need to export internal debug functions (e.g. curl_dbg_*), so disable
# symbol hiding for debug builds.
set(CURL_HIDDEN_SYMBOLS OFF)
Expand Down
9 changes: 2 additions & 7 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,8 @@ option(ENABLE_CURLDEBUG "Set to ON to build with TrackMemory feature enabled" OF
include(PickyWarnings)

if(ENABLE_DEBUG)
# DEBUGBUILD will be defined only for Debug builds
set_property(DIRECTORY APPEND PROPERTY COMPILE_DEFINITIONS $<$<CONFIG:Debug>:DEBUGBUILD>)
set(ENABLE_CURLDEBUG ON)
endif()

if(ENABLE_CURLDEBUG)
set_property(DIRECTORY APPEND PROPERTY COMPILE_DEFINITIONS CURLDEBUG)
add_definitions("-DDEBUGBUILD")
add_definitions("-DCURLDEBUG")
endif()

# For debug libs and exes, add "-d" postfix
Expand Down
2 changes: 1 addition & 1 deletion lib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ if(BUILD_TESTING)
target_compile_definitions(curlu PUBLIC UNITTESTS CURL_STATICLIB)
endif()

if(ENABLE_CURLDEBUG)
if(ENABLE_DEBUG)
# We must compile these sources separately to avoid memdebug.h redefinitions
# applying to them.
set_source_files_properties(memdebug.c curl_multibyte.c PROPERTIES SKIP_UNITY_BUILD_INCLUSION ON)
Expand Down
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ if(BUILD_STATIC_CURL)
set(CURLX_CFILES ${CURLTOOL_LIBCURL_CFILES})
endif()

if(ENABLE_CURLDEBUG)
if(ENABLE_DEBUG)
# We must compile this source separately to avoid memdebug.h redefinitions
# applying to them.
set_source_files_properties(../lib/curl_multibyte.c PROPERTIES SKIP_UNITY_BUILD_INCLUSION ON)
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ include_directories(
${CURL_BINARY_DIR}/include # To be able to reach "curl/curl.h"
)

if (ENABLE_CURLDEBUG) # running unittests require curl to compiled with CURLDEBUG
if(ENABLE_DEBUG) # running unittests require curl to compiled with CURLDEBUG
foreach(_testfile ${UNITPROGS})
add_executable(${_testfile} EXCLUDE_FROM_ALL ${_testfile}.c ${UNITFILES})
add_dependencies(testdeps ${_testfile})
Expand Down

0 comments on commit 130ddfa

Please sign in to comment.