Page MenuHomePhabricator

[BUILD] Search and include OpenSSL only where required
ClosedPublic

Authored by Fabien on May 25 2020, 12:34.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABC617ddd5be3c0: [BUILD] Search and include OpenSSL only where required
Summary

Since D6242 there is no more OpenSSL needed outside of the BIP70
feature. This diff cleans up the build system to reflect this.

Inspired by
https://github.com/bitcoin/bitcoin/pull/17165/commits/fcee10c2d028cba11416d902f5abf13fea7a65f4
and https://github.com/bitcoin/bitcoin/pull/17265/commits/8983ee3e6dd8ab658bd2caf97c326cc53ea50818,
but obviously different because we maintain BIP70.

Depends on D6251.

Test Plan
ninja all check-all bench-bitcoin

make
make check

Run the Gitian builds.

Uninstall OpenSSL (or make it fail to be found), then:

cmake -GNinja .. \
  -DENABLE_BIP70=OFF \
  -DSECP256K1_BUILD_OPENSSL_TESTS=OFF
ninja all check-all

With autotools, the whole secp256k1 test suite should be disabled in
order to skip openssl detection. Unfortunately the option name collides
with the bitcoin abc option to disable the tests, so they end up to be
all disabled.

../configure \
  --enable-deprecated-build-system \
  --disable-bip70 \
  --enable-tests=no
make

Event Timeline

Fabien requested review of this revision.May 25 2020, 12:34
Fabien planned changes to this revision.May 25 2020, 12:35
Fabien requested review of this revision.May 25 2020, 13:36

Gitian builds are green

deadalnix requested changes to this revision.May 25 2020, 16:07
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/config/CMakeLists.txt
218

Is that really the right place for this? It doesn't looks like it is affecting the config.

This revision now requires changes to proceed.May 25 2020, 16:07
Fabien requested review of this revision.May 25 2020, 16:31
Fabien added inline comments.
src/config/CMakeLists.txt
218

The definition is part of the config file, for compatibility with autotools.

Looking at the code it can easily be made a compiler definition instead of being part of the config file, but I'll do that in another diff.

deadalnix requested changes to this revision.May 25 2020, 17:09
This revision now requires changes to proceed.May 25 2020, 17:09
This revision is now accepted and ready to land.May 26 2020, 11:50