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 compile error with vs2017 and boost 1.67 #57

Closed
wants to merge 1 commit into from

Conversation

ShineQi
Copy link

@ShineQi ShineQi commented Jun 30, 2018

The default location of OpenSSL configuration changed to C:/Program Files/Common Files/SSL/openssl.cnf.
When compiling with boost 1.67 using vs2017, an error of c2039 saying native is not a member of the namespace boost::asio::basic_stream_socketboost::asio::ip::tcp, and the member native() should be native_handle()

@jmjatlanta
Copy link

jmjatlanta commented Jun 30, 2018

Thank you for this PR.

Boost 1.66 changed some exception handling, which Bitshares is not ready for. So versions above 1.65 will currently not work with bitshares-core.

In addition, I have not been able to get VS2017 to work with bitshares-core. VS2015 Update 1 is the latest compiler that I have heard works. @abitmore uses this with success.

I am still unable to complete the compilation on the Windows platform using VS2015 Update 1 and Boost 1.65 due to a configuration issue I have yet to find and fix. So this information is second-hand. I hope it helps!

Notes:
->native to ->native_handle was fixed at #36
but I missed one. Nice catch @cwyyprog.

Windows Bitshares howto: https://gist.github.com/jmjatlanta/0b1bf8f5cc0ebf893e0b5a5bb65e9262
The original document: https://github.com/abitmore/bts-cn-docs/blob/master/%E4%BD%BF%E7%94%A8VisualStudio2015%E7%BC%96%E8%AF%91BitShares-Core.txt

@@ -511,7 +511,11 @@ ENDIF()
IF("${OPENSSL_ROOT_DIR}" STREQUAL "")
get_filename_component(OPENSSL_ROOT_DIR "${OPENSSL_INCLUDE_DIR}/.." REALPATH)
ENDIF()
SET(OPENSSL_CONF_SOURCE "${OPENSSL_ROOT_DIR}/ssl/openssl.cnf")
IF("${OPENSSL_ROOT_DIR}" STREQUAL "C:/Program Files/OpenSSL")
SET(OPENSSL_CONF_SOURCE "C:/Program Files/Common Files/SSL/openssl.cnf")
Copy link
Member

Choose a reason for hiding this comment

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

Thank you anyway.

  • Hard-coded driver symbol and path are not good practice, because not everyone installs the files to the same path even though it's default behavior of installer. Better approach would be to add a new variable for location of openssl.cnf file, E.G. OPENSSL_CNF_FILE_DIR. Perhaps best approach is to detect whether the file exists in one location, if not, try another location.
  • A minor issue: need to replace the tabs in the code with white spaces.

@abitmore
Copy link
Member

abitmore commented Jul 1, 2018

I'd recommend that we split this pull request into 2 pull requests, since the "native" one is wanted now, but the other one needs more discussion.

@ShineQi
Copy link
Author

ShineQi commented Jul 5, 2018

I'll modify the PR as @abitmore saied

@ShineQi ShineQi closed this Jul 5, 2018
@ShineQi ShineQi deleted the cwyy branch April 13, 2019 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants