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

Jump to bottom

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
Closed

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

reddwarf69 opened this issue Nov 4, 2020 · 5 comments
Labels
cmake KNOWN_BUGS material

Comments

@reddwarf69
Copy link
Contributor

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 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 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

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 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 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
@bagder
KNOWN_BUGS: cmake build doesn't fail if zlib not found
4dbf349 
Closes #6173
@bagder bagder closed this as completed in 529423a Nov 18, 2020
bagder pushed a commit that referenced this issue Nov 19, 2020
@reddwarf69 @bagder
cmake: make CURL_ZLIB a tri-state variable
f21cc62 
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
Assignees
No one assigned
Labels
cmake KNOWN_BUGS material
Milestone
No milestone
Development

No branches or pull requests
4 participants
@snikulov @bagder @reddwarf69 @jzakrzewski