Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

refactor yubihsm: drop curl & pkgconfig build deps; libusb runtime dep #9075

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

spoonincode
Copy link
Contributor

Change Description

In 1.7 libyubihsm was integrated in to eosio's keosd making it significantly easier to use compared to tracking down a shared lib and placing it in the correct location. This also meant building eosio required a few new dependencies like pkgconfig, libusb, and libcurl. But these are very common packages so no problems, right?

Unfortunately yes, there has been some grief. For example, there was recently some pkgconfig+libcurl issues with our macos builds due to something in homebrew going awry. The way libyubihsm pulls in OpenSSL forces dynamic linking of it thus causing problems with macos hardened builds. libusb is LGPL which means if we ever wanted a "standalone" binary we couldn't static link libusb.

This PR resolves these issues and reduces the build and runtime depencies of eosio.

  • pkgconfig: no longer used. Instead, typical cmake constructs are used to find dependencies
  • libcurl: no longer used. the http code has been completely refactored to use boost beast instead of libcurl
  • libusb: Now optional at build time. If it's not found at build time a warning message is displayed via cmake and libyubihsm is built without usb support. If it is found at build time, USB support is enabled but eosio will not link to libusb. Instead, at runtime, a dlopen() is performed on libusb and it's used that way. This allows, for example, a standalone binary that doesn't balk at the absence of libusb on startup.

I constructed the changes such that upstream's repo can still be used without a fork. This means upstream's CMakeLists.txt is completely ignored and "reimplemented" locally. I realize there are advantages and disadvantages to this approach but it seemed the best fit to me.

I've removed libcurl and pkgconfig from cicd & script dependency installs. But for Ubuntu I needed to replace it with build-essential (which we probably should have been installing anyways).

Leaving as draft until I decide what to do about lack of unit testing. At a point ready for feedback though.

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@dimas1185
Copy link
Contributor

Have you considered using http_client from fc library in implementation? it is based on boost beast as well.
You can add it to the struct state and make all calls through it. We don't have tests for it either. However it is something already in use.
P.S.
FYI: There are plans for unified http(s) solution. During BlockVault work I was working on https server and client plugins based on boost beast. Than it was moved to SQL solution but server is ready for about 50-60%. And it does have tests. Actual plans to finish that and to make existing http_server_plugin as a proxy to new plugin or library. And if we will have enough reasons to do the same with http_client there are chances we will do same for client.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants