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

Conversation

Projects
None yet
3 participants
@vszakats
Contributor

vszakats commented Mar 2, 2016

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

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Mar 2, 2016

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

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 vszakats/curl-for-win that referenced this pull request Mar 2, 2016

curl: improvements [ci skip]
* 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 vszakats/curl-for-win that referenced this pull request Mar 2, 2016

curl: build improvements
* 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

This comment has been minimized.

Show comment
Hide comment
@jay

jay Mar 5, 2016

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@vszakats

vszakats Mar 5, 2016

Contributor

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

Contributor

vszakats commented Mar 5, 2016

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

@vszakats

This comment has been minimized.

Show comment
Hide comment
@vszakats

vszakats Mar 5, 2016

Contributor

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)

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@vszakats

vszakats Mar 5, 2016

Contributor

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

Contributor

vszakats commented Mar 5, 2016

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

@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Mar 5, 2016

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@vszakats

vszakats Mar 5, 2016

Contributor

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

Contributor

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 added a commit that referenced this pull request Mar 6, 2016

@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Mar 6, 2016

Member

Thanks, landed in 6c7a5b9.

Member

jay commented Mar 6, 2016

Thanks, landed in 6c7a5b9.

@jay jay closed this Mar 6, 2016

@vszakats vszakats deleted the vszakats:winssl branch Mar 6, 2016

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