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

Fix linking with -lcrypto when it is in non-standard path #149

Closed
wants to merge 1 commit into from

Conversation

koprok
Copy link

@koprok koprok commented Dec 10, 2018

No description provided.

@emweb
Copy link
Collaborator

emweb commented Apr 30, 2019

Maybe it's better if we just use CMake's FindOpenSSL instead?

@koprok
Copy link
Author

koprok commented Apr 30, 2019

@emweb, I agree it's better to use CMake's FindOpenSSL, but I wanted to be less intrusive and do it the way it's already done for the other libraries without changing very much, i.e. minimize the risk to break something.
What do you think about merging it this way and then open an issue to modernize CMakeLists to use find_package(OpenSSL) instead of WtFindSSL? I could do the changes later when I have the time and test them on my own system.

@emweb
Copy link
Collaborator

emweb commented Apr 30, 2019

I think that actually won't be necessary, since I already had a patch. I just didn't do anything with it yet.

If you want to take a look at it to make sure its ok (in my testing so far it seemed fine), here it is.

cmake_openssl.patch.txt

@koprok
Copy link
Author

koprok commented Apr 30, 2019

@emweb, I have tested your patch. Unfortunately it does not work for me. I am using Wt built through Vcpkg (https://github.com/Microsoft/vcpkg) on Linux. It is linked to SSL library that comes with Vcpkg instead of the system SSL library. I will try to find out what is wrong and get back to you.

@koprok
Copy link
Author

koprok commented May 2, 2019

@emweb, everything is fine with your patch for me! It was my mistake, the patch was not applied correctly over official Wt 4.0.5 version and I did not notice that initially. Please, include your patch in the next Wt version.

@emweb
Copy link
Collaborator

emweb commented May 6, 2019

I pushed the patch to master, so it will be in the next release.

Regards,
Roel

@emweb emweb closed this May 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants