Skip to content

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Sep 28, 2025

Also:

  • fix to append to, not override, previously set linker options when
    using CURL_LIBCURL_VERSIONED_SYMBOLS=ON. Before this patch, it was
    overwriting linker options when using CURL_CODE_COVERAGE=ON.

    Follow-up to 91720b6 cmake: add CURL_CODE_COVERAGE option #18468


  • consider LINK_FLAGS in place of target_link_libraries() for old cmake

This started out as this modification, before settling to be a simple
tidy-up:

cmake: fix passing compiler options to cmake 3.10 and older

COMPILE_OPTIONS requires 3.11+:
https://cmake.org/cmake/help/v3.19/prop_sf/COMPILE_OPTIONS.html

Replace it with target_compile_options(), and add_compile_options():
https://cmake.org/cmake/help/v3.7/command/target_compile_options.html
https://cmake.org/cmake/help/v3.7/command/add_compile_options.html

Another alternative would be to use COMPILE_FLAGS, but it requires
a space-separated string of options, which is inconvenient:
https://cmake.org/cmake/help/v3.7/prop_sf/COMPILE_FLAGS.html

Also:

  • replace COMPILE_FLAGS with modern alternatve when possible,
    and switch to target_compile_options() for old versions.
  • replace target_link_options() with LINK_OPTIONS on modern
    cmake.

Follow-up to 91720b6 #18468
Follow-up to 80f9dd0 #17523
Follow-up to e865420 #17047
Follow-up to 45f7cb7 #16238


w/o sp https://github.com/curl/curl/pull/18762/files?w=1

  • think it over once again what would be the least bad solution.
    E.g. target_compile_options() could be used for new cmake too,
    but goes against the general direction to use props.
    Perhaps also stick with target_link_options() for 3.13+. [NOT NEEDED]

@vszakats vszakats added the cmake label Sep 28, 2025
@vszakats vszakats marked this pull request as draft September 28, 2025 01:31
@vszakats
Copy link
Member Author

vszakats commented Sep 28, 2025

According to 81a9197 #18764:

cd /__w/curl/curl/bld-1/lib && /usr/bin/cc  -DBUILDING_LIBCURL -DCURL_HIDDEN_SYMBOLS -DHAVE_CONFIG_H -D_GNU_SOURCE -Dlibcurl_shared_EXPORTS -I/__w/curl/curl/include -isystem /usr/include/p11-kit-1 -I/__w/curl/curl/bld-1/lib  -I/usr/include/p11-kit-1 -I/usr/include/p11-kit-1 -fPIC   -fvisibility=hidden -Werror -pedantic-errors -Werror-implicit-function-declaration -W -Wall -pedantic -Wbad-function-cast -Wconversion -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-long-long -Wno-multichar -Wpointer-arith -Wshadow -Wsign-compare -Wundef -Wunused -Wwrite-strings -Waddress -Wattributes -Wcast-align -Wcast-qual -Wdeclaration-after-statement -Wdiv-by-zero -Wempty-body -Wendif-labels -Wfloat-equal -Wformat-security -Wignored-qualifiers -Wmissing-field-initializers -Wmissing-noreturn -Wno-format-nonliteral -Wno-padded -Wno-sign-conversion -Wno-switch-default -Wno-switch-enum -Wno-system-headers -Wold-style-definition -Wredundant-decls -Wstrict-prototypes -Wtype-limits -Wunreachable-code -Wunused-parameter -Wvla -Wclobbered -Wmissing-parameter-type -Wold-style-declaration -Wpragmas -Wstrict-aliasing=3 -ftree-vrp -Wjump-misses-init -Wdouble-promotion -Wformat=2 -Wtrampolines -Warray-bounds=2 -Wduplicated-cond -Wnull-dereference -fdelete-null-pointer-checks -Wshift-negative-value -Wshift-overflow=2 -Wunused-const-variable -o CMakeFiles/libcurl_shared.dir/dynhds.c.o   -c /__w/curl/curl/lib/dynhds.c

Ref: https://github.com/curl/curl/actions/runs/18072632773/job/51424653376#step:5:559

It surprised me is that 3.7.2 does seem to understand the
COMPILE_OPTIONS directory property, even though this is documented
to have appeared in 3.11 (this notice appearing in the documentation in 3.19):
https://cmake.org/cmake/help/v3.11/prop_sf/COMPILE_OPTIONS.html
Since e865420 (2025-04-14), we
pass picky compiler warning options via COMPILE_OPTIONS, yet they
also appear in the old-linux verbose output, that's using 3.7.2. It will need
more testing to see if this is universally true for every COMPILE_OPTIONS
use in curl's cmake scripts.

COMPILE_OPTIONS was committed here:
Kitware/CMake@80ca9c4
First appeared in 2.8.12 (2013-Oct-8). This was before target_compile_options()
and add_compile_options() were added, which in turn uses COMPILE_OPTIONS
internally and both appeared also in 2.8.12:
add_compile_options(): Kitware/CMake@a984f32
target_compile_options(): Kitware/CMake@24466f2

I'm not sure why it wasn't documented till 3.11, or if there is more subtlety here.

Ref: #18764 (comment):

FTR LINK_OPTIONS was added via (also to the documentation): Kitware/CMake@c1f5a44

@vszakats
Copy link
Member Author

vszakats commented Sep 28, 2025

Right, it's documented for 3.7.x on these links:
https://cmake.org/cmake/help/v3.7/prop_tgt/COMPILE_OPTIONS.html
https://cmake.org/cmake/help/v3.7/prop_dir/COMPILE_OPTIONS.html

3.11 likely added support for it as source file properties.

@vszakats vszakats changed the title cmake: fix passing compiler options to cmake 3.10 and older cmake: use more COMPILER_OPTIONS, LINK_OPTIONS / LINK_FLAGS Sep 28, 2025
@vszakats vszakats marked this pull request as ready for review September 28, 2025 23:01
@vszakats vszakats closed this in e17aa98 Sep 29, 2025
@vszakats vszakats deleted the cmold branch September 29, 2025 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant