Differential D6243

[BUILD] Search and include OpenSSL only where required
ClosedPublic
Actions

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

Diff Detail

Repository

rABC Bitcoin ABC

Branch

PR17265_part4

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 11010
Build 20312: Default Diff Build & Tests
Build 20311: Build without Wallet
Build 20310: Build with clang-10
Build 20309: arc lint + arc unit

Event Timeline

Fabien created this revision.May 25 2020, 12:34

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 25 2020, 12:34

Fabien requested review of this revision.May 25 2020, 12:34

Build Bitcoin ABC Diffs / Diff Testing (build-werror) started.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) started.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) started.

Fabien planned changes to this revision.May 25 2020, 12:35

Build Bitcoin ABC Diffs / Diff Testing (build-werror) passed.

Build Bitcoin ABC Diffs / Gitian Diff Testing (linux) started.

Build Bitcoin ABC Diffs / Gitian Diff Testing (osx) started.

Build Bitcoin ABC Diffs / Gitian Diff Testing (win) started.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) passed.

Build Bitcoin ABC Diffs / Gitian Diff Testing (osx) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B11010: Diff 20582.May 25 2020, 12:46

Build Bitcoin ABC Diffs / Gitian Diff Testing (win) passed.

Build Bitcoin ABC Diffs / Gitian Diff Testing (linux) passed.

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.