Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix Ubuntu 18.04 LTS Build #3119

Merged
merged 6 commits into from
Oct 30, 2018
Merged

Conversation

mvandeberg
Copy link
Contributor

Closes #3102

This fixes here can be split in to two categories:

  1. Fixes because of updating to OpenSSL 1.1
  2. Warnings reported by a newer version of gcc

This biggest change because of OpenSSL is the need to upgrade our version of Websocketpp to 0.8.1 (https://github.com/zaphoyd/websocketpp/releases/tag/0.8.1)

@theoreticalbts
Copy link
Contributor

theoreticalbts commented Oct 29, 2018

  • In my personal coding style, wip or fixup is code that I intend to rebase. I don't like to let commits with these labels leak into released code without undergoing changes (usually there is something wrong / incomplete / untested with such code, but if it's okay as-is, go ahead and use reword or fixup option of git rebase -i)
  • Why is miniz.c changed? Adding style doesn't seem like a good reason to change a vendored file.
  • What Bitcoin version are the base58 files from? Why not use the latest version?

@mvandeberg
Copy link
Contributor Author

In my personal coding style, wip or fixup is code that I intend to rebase. I don't like to let commits with these labels leak into released code without undergoing changes (usually there is something wrong / incomplete / untested with such code, but if it's okay as-is, go ahead and use reword or fixup option of git rebase -i)

I used your branch as a starting point. Some of the changes are used. Others are not. I can rebase to change your commit messages if you'd like.

Why is miniz.c changed? Adding style doesn't seem like a good reason to change a vendored file.

It failed because of gcc warnings. Changing style allows us to proceed using -Werror.

What Bitcoin version are the base58 files from? Why not use the latest version?

This is the BitShares OpenSSL 1.1 fix (linked to in the issue). Using latest Bitcoin base58 implementation would require adding additional dependencies.

@mvandeberg mvandeberg force-pushed the 2018-10-22-ubuntu-18-04-build-fix branch from e7a0e2e to 071f6a9 Compare October 30, 2018 16:29
@theoreticalbts
Copy link
Contributor

LGTM

@mvandeberg mvandeberg merged commit 75300af into master Oct 30, 2018
@mvandeberg mvandeberg deleted the 2018-10-22-ubuntu-18-04-build-fix branch October 30, 2018 23:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix build in Ubuntu 18.04 LTS
2 participants