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

Add zlib to libs when finding openssl #2187

Closed
wants to merge 1 commit into from
Closed

Add zlib to libs when finding openssl #2187

wants to merge 1 commit into from

Conversation

mellery451
Copy link
Contributor

FIXES: (RIPD-1496):

if openssl is configured with compression support AND you link
to the static lib, you are going to need to link to zlib. Fundamentally
the CMake finder should take care of this (as described in
https://gitlab.kitware.com/cmake/cmake/issues/16885), but we
work around it here by adding zlib explicitly.

@codecov-io
Copy link

codecov-io commented Jul 22, 2017

Codecov Report

Merging #2187 into develop will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2187      +/-   ##
===========================================
- Coverage    69.96%   69.94%   -0.02%     
===========================================
  Files          689      689              
  Lines        50734    50734              
===========================================
- Hits         35495    35488       -7     
- Misses       15239    15246       +7
Impacted Files Coverage Δ
src/ripple/basics/DecayingSample.h 77.77% <0%> (-8.34%) ⬇️
src/ripple/app/ledger/PendingSaves.h 93.54% <0%> (-6.46%) ⬇️
src/ripple/protocol/impl/STVar.cpp 85.71% <0%> (-2.6%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cca574c...cd8c6c3. Read the comment docs.

@mellery451 mellery451 requested review from ximinez and wilsonianb July 24, 2017 14:48
@wilsonianb
Copy link
Contributor

This allows me to successfully build rippled on Fedora26 with openssl-static and zlib-static installed.
However the beast.asio.error test fails.
#2047 (comment)
#2151

Copy link
Contributor

@wilsonianb wilsonianb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍
Thanks!

Copy link
Collaborator

@ximinez ximinez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. Windows doesn't support static builds, but the non-static builds are fine. I abused several different combinations of options under linux, and they all worked fine as well. 👍

I noticed the Travis build failed, so I restarted it. However, I don't anticipate it succeeding. Please rebase to the latest beta, which has Travis fixes, so we can confirm CI success.

@wilsonianb wilsonianb added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Aug 8, 2017
FIXES: (RIPD-1496):

if openssl is configured with compression support AND you link
to the static lib, you are going to need to libk to zlib. Fundamentally
the CMake finder should take care of this (as described in
https://gitlab.kitware.com/cmake/cmake/issues/16885), but we
work around it here by adding zlib explicitly. Update beast error test
for OpenSSL 1.1.0
@nbougalis
Copy link
Contributor

Merged as 36423a5

@nbougalis nbougalis closed this Aug 15, 2017
@mellery451 mellery451 deleted the zlib branch August 15, 2017 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants