[CMAKE] Allow for building static and shared versions of secp256k1
AbandonedPublic
Actions

Authored by Fabien on Aug 14 2019, 11:58.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Summary

The previous behavior generated a static library by default, and allowed
for building a shared library by setting -DBUILD_SHARED_LIBS=ON on the
cmake command line when building secp256k1 standalone.
It was not possible to build both a static and a shared library at the
same time, which would unfortunately prevent running the secp256k1 Java
tests as part of the check-all target, because these tests require a
shared secp256k1 library.

This diff is a first step toward building the JNI binding and running
the Java tests. It still defaults to building a static secp256k1 library
and add an option (SECP256K1_BUILD_SHARED) for building the shared
version also.

Test Plan

mkdir buildcmake && cd buildcmake
cmake -GNinja .. -DSECP256K1_BUILD_SHARED=ON
ninja
ninja check-all
ninja bench-secp256k1

Diff Detail

Repository

rABC Bitcoin ABC

Branch

cmake_secp256k1_object_lib

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 7141
Build 12327: Bitcoin ABC Buildbot (legacy)
Build 12326: arc lint + arc unit

Event Timeline

Fabien created this revision.Aug 14 2019, 11:58

Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 14 2019, 11:58

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Diff Testing started.

Fabien added a child revision: D3871: [CMAKE] Add support to build secp256k1 JNI binding and tests.Aug 14 2019, 12:00

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7141: Diff 10780.Aug 14 2019, 12:05

I'm not a fan of the approach. There is a set of global settings for cmake that allow to specify if you want thing shared or not by default. secp256k1 should be whatever the default is for cmake.

If a hared version of it is necessary, it is possible to add a second library, that is always shared.

src/secp256k1/CMakeLists.txt
180	You shouldn't hide the definition behind a option. If JNI test require it, they should find the definition. It is however possible to exclude the library from the `all` target or not, based on the option.