cmake: delete obsolete TODO items [ci skip]#12500
Closed
vszakats wants to merge 1 commit intocurl:masterfrom
Closed
cmake: delete obsolete TODO items [ci skip]#12500vszakats wants to merge 1 commit intocurl:masterfrom
vszakats wants to merge 1 commit intocurl:masterfrom
Conversation
There is always room for improvement, but CMake is up to par now with autotools, so there is no longer a good reason to keep around these inline TODO items. Answering one of questions: Q: "The gcc command line use neither -g nor any -O options. As a developer, I also treasure our configure scripts's --enable-debug option that sets a long range of "picky" compiler options." A: CMake offers the `CMAKE_BUILD_TYPE` variable to control debug info and optimization level. E.g.: - `Release` = `-O3` + no debug info - `MinSizeRel` = `-Os` + no debug info - `Debug` = `-O0` + debug info https://stackoverflow.com/questions/48754619/what-are-cmake-build-type-debug-release-relwithdebinfo-and-minsizerel/59314670#59314670 https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#default-and-custom-configurations For picky warnings we have the `PICKY_COMPILER` options, enabled by default. Closes #xxxxx
bagder
approved these changes
Dec 11, 2023
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Dec 12, 2023
- 898b012 curl#1288 - 5de6848 curl#10023 - bunch of others that are completed - `NTLM_WB_ENABLED` is implemented in a basic form, and now also scheduled for removal, so a TODO at this point isn't useful. And this 'to-check' item: Q: "The cmake build selected to run gcc with -fPIC on my box while the plain configure script did not." A: With CMake, since 2ebc74c curl#11546 and fc9bfb1 curl#11627, we explicitly enable PIC for libcurl shared lib. Or when building libcurl for shared and static lib in a single pass. We do this by default for Windows or when enabled by the user via `SHARE_LIB_OBJECT`. Otherwise we don't touch this setting. Meaning the default set by CMake (if any) or the toolchain is used. On Debian Bookworm, this means that PIC is disabled for static libs by default. Some platforms (like macOS), has PIC enabled by default. autotools supports the double-pass mode only, and in that case CMake seems to match PIC behaviour now (as tested on Linux with gcc.) Follow-up to 5d5dfdb curl#12500 Closes #xxxxx
vszakats
added a commit
that referenced
this pull request
Dec 13, 2023
- manual completed: 898b012 #1288 - soname completed: 5de6848 #10023 - bunch of others that are completed - `NTLM_WB_ENABLED` is implemented in a basic form, and now also scheduled for removal, so a TODO at this point isn't useful. And this 'to-check' item: Q: "The cmake build selected to run gcc with -fPIC on my box while the plain configure script did not." A: With CMake, since 2ebc74c #11546 and fc9bfb1 #11627, we explicitly enable PIC for libcurl shared lib. Or when building libcurl for shared and static lib in a single pass. We do this by default for Windows or when enabled by the user via `SHARE_LIB_OBJECT`. Otherwise we don't touch this setting. Meaning the default set by CMake (if any) or the toolchain is used. On Debian Bookworm, this means that PIC is disabled for static libs by default. Some platforms (like macOS), has PIC enabled by default. autotools supports the double-pass mode only, and in that case CMake seems to match PIC behaviour now (as tested on Linux with gcc.) Follow-up to 5d5dfdb #12500 Reviewed-by: Jay Satiro Closes #12509
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There is always room for improvement, but CMake is up to par now with autotools, so there is no longer a good reason to keep around these inline TODO items.
Answering one of questions:
Q: "The gcc command line use neither -g nor any -O options. As a
developer, I also treasure our configure scripts's --enable-debug
option that sets a long range of "picky" compiler options."
A: CMake offers the
CMAKE_BUILD_TYPEvariable to control debug infoand optimization level. E.g.:
Release=-O3+ no debug infoMinSizeRel=-Os+ no debug infoDebug=-O0+ debug infohttps://stackoverflow.com/questions/48754619/what-are-cmake-build-type-debug-release-relwithdebinfo-and-minsizerel/59314670#59314670
https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#default-and-custom-configurations
For picky warnings we have the
PICKY_COMPILERoptions, enabled bydefault.
Closes #12500