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

cmake: doesn't fail if zlib support is requested but the zlib library can't be found #6173

Closed
RedDwarf69 opened this issue Nov 4, 2020 · 5 comments

Comments

@RedDwarf69
Copy link
Contributor

@RedDwarf69 RedDwarf69 commented Nov 4, 2020

Running cmake -DCURL_ZLIB:BOOL=ON succeeds even if find_package(ZLIB) fails.

In general the checks are inconsistent. With find_package(Brotli QUIET) (https://github.com/curl/curl/blob/master/CMakeLists.txt#L653) immediately followed by find_package(Zstd REQUIRED) (https://github.com/curl/curl/blob/master/CMakeLists.txt#L665).

On autotools you have --with-zlib/--without-zlib and autodetection. If --with-zlib is used the configure script fails if the zlib library can't be found.

@bagder bagder added the cmake label Nov 4, 2020
@snikulov
Copy link
Member

@snikulov snikulov commented Nov 5, 2020

@bagder should we completely mimic autotools? As I see it was the initial intention (12 years or so 600460f) not to fail if CURL_ZLIB=ON, but zlib is not found.
Should we re-work CMakeLists.txt now to match autotools?

@bagder
Copy link
Member

@bagder bagder commented Nov 5, 2020

I don't think we need to "blindly" follow and mimic autotools in cmake (as it certainly has its flaws and peculiarities as well), but the things that are done "right" and that we like we certainly could.

I think when a user asks for enabling a feature, like zlib, we should fail if that can't be done - using any build system. If the user doesn't ask for it, then detecting it automatically and using it or not based on that seems reasonable. Right?

@jzakrzewski
Copy link
Contributor

@jzakrzewski jzakrzewski commented Nov 6, 2020

I'm with @bagder . CMake build should probably behave "the CMake way" mimicking autotools where it makes sense.
But in any case in autodetection mode we should be able to decide what to do if dependency is not found (like skip it) but if the user asks for a particular option and it cannot be delivered, that's a hard error.

@snikulov
Copy link
Member

@snikulov snikulov commented Nov 6, 2020

@jzakrzewski I'm also not arguing. Then we should re-write all CMake build to new rules.

@bagder
Copy link
Member

@bagder bagder commented Nov 6, 2020

It's good enough if we keep fixing things, one by one. Just like we always have!

bagder added a commit that referenced this issue Nov 18, 2020
Closes #6173
@bagder bagder closed this in 529423a Nov 18, 2020
bagder added a commit that referenced this issue Nov 19, 2020
By differentiating between ON and AUTO it can make a missing zlib
library a hard error when CURL_ZLIB=ON is used.

Reviewed-by: Jakub Zakrzewski
Closes #6221
Fixes #6173
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.