ladnir / cryptoTools

A repo to hold common tools used by my crypto projects
Other
117 stars 53 forks source link

Optimize compilation and fix minor errors #28

Closed chuckchen closed 3 years ago

chuckchen commented 3 years ago

First things first, thanks a lot for this great library!

This PR adds third-party dependencies including Boost, Relic and Sodium into the build tree. The Python scripts for downloading and building those dependencies are no longer needed. What's more, by switching to CMake ExternalProject_Add for dependency management, it's more consistent to pass build options around.

Notable changes include:

  1. Add Boost, Relic and Sodium as external projects, set up dependencies during configuring
  2. Change Relic and Sodium to static linking
  3. Explicitly include proper header files

[edited for new commit]

ladnir commented 3 years ago

What is the motivation for this? I was happy with managing dependencies using the python scripts.

There needs to be a good reason to change due to all the downstream work that would be required.

chuckchen commented 3 years ago

I understand the python scripts are fully functional and were the standard build process. Some developers may find it more idiomatic building with just CMake.

This change may still respect python script based way, and leverage new CMake features to streamline the building process. Actually, the installation directory can be shared between those 2 approaches.

ladnir commented 3 years ago

What is the mechanism for deciding when to download the dependancies? It's just automatic?

Does it support system installed dependancies?

chuckchen commented 3 years ago

It's just automatic, and supports system wide dependencies. The process boils down to the following steps:

  1. If any of ENABLE_RELIC/ENABLE_SODIUM/ENABLE_BOOST is set, try find in system path first, followed by project thirdparty installation directory, since ${OC_THIRDPARTY_HINT} is appended to CMAKE_PREFIX_PATH.
  2. If not found, download the tar file or pull git tags configured through ExternalProject_Add, and build dependencies with CMake. Compiling options are the same with those in getBoost.py/getRelic.py/getSodium.py.
  3. Try find libraries again in both system path and thirdparty installation path. If found, set up libraries just like the good old way. Or complain and exit.

The commit has been cleaned up to exclude irrelevant changes, and has been tested on both macOS and Linux.

ladnir commented 3 years ago

OK, I'll merge this. I'm a bit busy for the next two weeks and want to run a few tests myself. Once I get some free time I'll merge.

Thanks for the PR.

ladnir commented 3 years ago

I got a chance to review the PR and agree it would be nice to handle everything in cmake. I think I will migrate in this direction but I dont think ill merge this PR into master. I'll try to push something out (building on your PR) in the next few weeks.

My main concern is that I want there to be just one solution (not this and python) and it should work across win, mac, Linux and the CI should pass. I also don't like the "download by default" method and will add a flag that must be set in order to automatically fetch the dependancies.

chuckchen commented 3 years ago

Okay. Looking forward to the changes.

ladnir commented 3 years ago

should be updated now. I only tested it with libOTe but hopefully, cryptoTools also works on it's own. Will do some more testing soon.

ladnir commented 3 years ago

if you are using the cmake interface, then you need to define -D FETCH_AUTO=ON to get it to auto download and build. See the output of cmake for more options.