tidy-up: delete parallel/unused feature flags #9652
Closed
Conversation
This file contains 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
Before this patch there existed two parallel way to enable zlib features: `HAVE_LIBZ` and `HAVE_ZLIB_H`. They had to be toggled in sync for a correct build. There was dedicated CMake logic making sure to ensure that. In zlib's case there just a single header file we include in curl, so an extra check for it is redundant. Let's keep HAVE_LIBZ and drop HAVE_ZLIB_H. There was also a USE_ZLIB variant used for MS-DOS. This patch retains it for compatibility and deprecates it. Let me know if we can delete it anyway from `lib/config-dos.h`.
A similar (but simpler) case to zlib. There were some outlier macros defined in `winbuild/MakefileBuild.vc`. Added recently in 2cadc89, but could not find any reference to them in either the curl source, in libssh sources (as of 0.9.6, 0.10.4) or via GitHub/internet search (except `CURL_USE_LIBSSH` which is curl-specific but not used in curl). These: `HAVE_LIBSSH`, `LIBSSH_WIN32`, `LIBSSH_LIBRARY` Turns out these were the result of copy-pasting the existing line for enabling libssh2. They can be safely deleted.
A similar case to zlib and libssh. Also delete some outlier macros from `winbuild/MakefileBuild.vc`: `HAVE_LIBSSH2`, `LIBSSH2_WIN32`, `LIBSSH2_LIBRARY` The first is not use in curl, the other two are libssh2-specific, but only necessary when building libssh2 itself. Also delete `HAVE_LIBSSH2` from CMake logic, by using the parallel `USE_LIBSSH2`.
vszakats
changed the title
bagder
approved these changes
Oct 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Detecting headers and lib separately makes sense when headers come in
variations or with extra ones, but this wasn't the case here. These were
duplicate/parallel macros that we had to keep in sync with each other
for a working build. This patch leaves a single macro for each of these
dependencies:
Rely on
HAVE_LIBZ, delete parallelHAVE_ZLIB_H.Also delete CMake logic making sure these two were in sync, along with
a toggle to turn off that logic, called
CURL_SPECIAL_LIBZ.Also delete stray
HAVE_ZLIBdefines.There is also a
USE_ZLIBvariant inlib/config-dos.h. This patchretains it for compatibility and deprecates it.
Rely on
USE_LIBSSH2, delete parallelHAVE_LIBSSH2_H.Also delete
LIBSSH2_WIN32,LIBSSH2_LIBRARYfromwinbuild/MakefileBuild.vc, these have a role when building libssh2itself. And
CURL_USE_LIBSSH, which had no use at all.Also delete stray
HAVE_LIBSSH2defines.Rely on
USE_LIBSSH, delete parallelHAVE_LIBSSH_LIBSSH_H.Also delete
LIBSSH_WIN32,LIBSSH_LIBRARYandHAVE_LIBSSHfromwinbuild/MakefileBuild.vc, these were the result of copy-pasting thelibssh2 line, and were not having any use.
Delete unused
HAVE_LIBPSL_HandHAVE_LIBPSL.Reviewed-by: Daniel Stenberg
Closes #9652