/curl

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 // build issue when OpenSSL enabled on Windows and schannel detected #617

Closed
wants to merge 1 commit into
base: master
from

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
build cmake
Projects
None yet
Milestone
No milestone
4 participants
@snikulov @mention-bot @bradking @bagder
@snikulov
Copy link
Member

snikulov commented Jan 27, 2016

Options OpenSSL and Windows Schannel should be exclusive.
Enabling them both will break the build - we got redefinition in ssl_connect_data structure (ref to Appveyour buils).

So on Windows I disable CMAKE_USE_OPENSSL by default (Windows Schannel is preferred).
But we always can override this option with
cmake .. -DCMAKE_USE_OPENNSSL=ON | OFF
@mention-bot

This comment has been minimized.

Sign in to view
Copy link

mention-bot commented Jan 27, 2016

By analyzing the blame information on this pull request, we identified @jzakrzewski, @billhoffman and @Sukender to be potential reviewers

@snikulov snikulov changed the title Fixed build issue when OpenSSL enabled on Windows and schannel detected cmake // build issue when OpenSSL enabled on Windows and schannel detected Jan 27, 2016

@snikulov snikulov referenced this pull request Jan 27, 2016

Closed

optimization // moved common variable out of #ifdef #618

@snikulov snikulov force-pushed the snikulov:fix_openssl_schannel_clash branch Jan 27, 2016

@snikulov

This comment has been minimized.

Sign in to view
Copy link
Member Author

snikulov commented Jan 27, 2016

@bradking Could you please review?
@bradking

bradking reviewed Jan 27, 2016
View changes

CMakeLists.txt Outdated
mark_as_advanced(CMAKE_USE_OPENSSL)

if(WIN32)
CMAKE_DEPENDENT_OPTION(CURL_WINDOWS_SSPI "Use windows libraries to allow NTLM authentication without openssl" OFF
"CMAKE_USE_OPENSSL" ON)

This comment has been minimized.

Sign in to view
Copy link
@bradking

bradking Jan 27, 2016

Contributor

IIUC the goal is to present the CURL_WINDOWS_SSPI option (with default ON) when CMAKE_USE_OPENSSL is OFF (default). When CMAKE_USE_OPENSSL is ON then CURL_WINDOWS_SSPI should be non-optional and forced to OFF. In that case the dependent option should be:

  CMAKE_DEPENDENT_OPTION(CURL_WINDOWS_SSPI "Use windows libraries to allow NTLM authentication without openssl" ON
    "NOT CMAKE_USE_OPENSSL" OFF)
@snikulov

This comment has been minimized.

Sign in to view
Copy link
Member Author

snikulov commented Jan 28, 2016

@bradking updated per your comments with snikulov@b2e6d0a

If it looks OK for you - I'll squash it into single commit.
@bradking

This comment has been minimized.

Sign in to view
Copy link
Contributor

bradking commented Jan 28, 2016

Yes, LGTM.

@bagder bagder added build cmake labels Jan 28, 2016

@snikulov snikulov force-pushed the snikulov:fix_openssl_schannel_clash branch to 9a7486e Jan 28, 2016

@snikulov

This comment has been minimized.

Sign in to view
Copy link
Member Author

snikulov commented Jan 28, 2016

@bagder I'm done.
CMake build for windows goes green again.

@bagder bagder closed this in a8135f0 Jan 28, 2016

@bagder

This comment has been minimized.

Sign in to view
Copy link
Member

bagder commented Jan 28, 2016

thanks!

@snikulov snikulov referenced this pull request Jan 28, 2016

Closed

updated AppVeyor build to handle OpenSSL/WinSSL builds #621

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.