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

makefile.m32: add missing libs for -winssl option #693

Closed
wants to merge 1 commit into from
Closed

makefile.m32: add missing libs for -winssl option #693

wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Mar 2, 2016

Required for a successful link. Similar logic is already present in libssh2.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @gknauf, @captain-caveman2k and @bagder to be potential reviewers

vszakats added a commit to curl/curl-for-win that referenced this pull request Mar 2, 2016
* generate .def file and use it when building the .dll,
  making sure to limit the list of exported functions
  to libcurl ones. This should fix the bloated .dll
  and implib.
* allow to build without nghttp2
* build with WinSSL if no other SSL backend is found
* document the reason for -DCURL_LIBSTATIC option
* fix building with WinSSL by using two local curl patches

curl patches were submitted upstream as:
curl/curl#692
curl/curl#693
vszakats added a commit to curl/curl-for-win that referenced this pull request Mar 2, 2016
* generate .def file and use it when building the .dll,
  making sure to limit the list of exported functions
  to libcurl ones. This should fix the bloated .dll
  and implib.
* allow to build without nghttp2
* build with WinSSL if no other SSL backend is found
* document the reason for -DCURL_LIBSTATIC option
* fix building with WinSSL by using two local curl patches

curl patches were submitted upstream as:
curl/curl#692
curl/curl#693
@jay
Copy link
Member

jay commented Mar 5, 2016

Required for a successful link.

Is this for a static build and are you sure the dependency on bcrypt is for WinSSL and not libssh2? You may need to wrap this in !DYN wherever it goes

@vszakats
Copy link
Member Author

vszakats commented Mar 5, 2016

Good points. I'll make tests and update accordingly. (Sorry for the sloppiness.)

@vszakats
Copy link
Member Author

vszakats commented Mar 5, 2016

Both of them is required for libssh2 only:
https://ci.appveyor.com/project/vszakats/harbour-deps/build/1.0.709#L1094

Tested with libssh2:
https://ci.appveyor.com/project/vszakats/harbour-deps/build/1.0.711

Patch updated.

(There are some libssh2 warnings which are valid and need to fixed not just for WinSSL, but for f.e. OpenSSL 1.1.0 as well)

@vszakats
Copy link
Member Author

vszakats commented Mar 5, 2016

Hold on for an hour. I'll recheck the .dll part, too.

@jay
Copy link
Member

jay commented Mar 5, 2016

There's no rush there's like 3 weeks until release, just update the thread when you have something ready and someone will take a look

@vszakats
Copy link
Member Author

vszakats commented Mar 5, 2016

Final candidate committed, with a simplification and tested with both .exe and .dll.

With libssh2: https://ci.appveyor.com/project/vszakats/harbour-deps/build/1.0.715
Without libssh2: https://ci.appveyor.com/project/vszakats/harbour-deps/build/1.0.716

jay pushed a commit that referenced this pull request Mar 6, 2016
@jay
Copy link
Member

jay commented Mar 6, 2016

Thanks, landed in 6c7a5b9.

@jay jay closed this Mar 6, 2016
@vszakats vszakats deleted the winssl branch March 6, 2016 09:09
@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.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants