Skip to content
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

tidy-up: delete parallel/unused feature flags #9652

Closed
wants to merge 6 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Oct 5, 2022

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 parallel HAVE_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_ZLIB defines.

    There is also a USE_ZLIB variant in lib/config-dos.h. This patch
    retains it for compatibility and deprecates it.

  • Rely on USE_LIBSSH2, delete parallel HAVE_LIBSSH2_H.

    Also delete LIBSSH2_WIN32, LIBSSH2_LIBRARY from
    winbuild/MakefileBuild.vc, these have a role when building libssh2
    itself. And CURL_USE_LIBSSH, which had no use at all.

    Also delete stray HAVE_LIBSSH2 defines.

  • Rely on USE_LIBSSH, delete parallel HAVE_LIBSSH_LIBSSH_H.

    Also delete LIBSSH_WIN32, LIBSSH_LIBRARY and HAVE_LIBSSH from
    winbuild/MakefileBuild.vc, these were the result of copy-pasting the
    libssh2 line, and were not having any use.

  • Delete unused HAVE_LIBPSL_H and HAVE_LIBPSL.

Reviewed-by: Daniel Stenberg

Closes #9652

vszakats added 4 commits Oct 5, 2022
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 vszakats added the build label Oct 5, 2022
@vszakats vszakats changed the title delete parallel/unused feature flags tidy-up: delete parallel/unused feature flags Oct 5, 2022
bagder
bagder approved these changes Oct 6, 2022
vszakats added a commit to curl/curl-for-win that referenced this pull request Oct 6, 2022
@vszakats vszakats closed this in 0c32746 Oct 6, 2022
@vszakats vszakats deleted the havedupe branch Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants