Merge bitcoin-core/secp256k1#1230: Build: allow static or shared but …

…not both

ef49a11 build: allow static or shared but not both (Cory Fields)
36b0adf build: remove warning until it's reproducible (Cory Fields)

Pull request description:

  Continuing from here: bitcoin-core/secp256k1#1224 (comment)

  Unfortunately it wasn't really possible to keep a clean diff here because of the nature of the change. I suggest reviewing the lib creation stuff in its entirety, sorry about that :\

  Rather than allowing for shared and static libs to be built at the same time like autotools, this PR switches to the CMake convention of allowing only 1.

  A new `BUILD_SHARED_LIBS` option is added to match CMake convention, as well as a `SECP256K1_DISABLE_SHARED` option which overrides it. That way even projects which have `BUILD_SHARED_LIBS=1` can opt-into a static libsecp in particular.

  Details:

  Two object libraries are created: `secp256k1_asm` and `secp256k1_precomputed_objs`. Some tests/benchmarks use the object libraries directly, some link against the real lib: `secp256k1`.

  Because the objs don't know what they're going to be linked into, they need to be told how to deal with PIC.

  The `DEFINE_SYMBOL` property sets the `DLL_EXPORT` define as necessary (when building a shared lib)

ACKs for top commit:
  hebasto:
    re-ACK ef49a11, only [suggested](bitcoin-core/secp256k1#1230 (review)) changes since my recent [review](bitcoin-core/secp256k1#1230 (review)).
  real-or-random:
    ACK ef49a11

Tree-SHA512: 8870de305176fdb677caff0fdfc6f8c59c0e906489cb72bc9980e551002812685e59e20d731f2a82e33628bdfbb7261eafd6f228038cad3ec83bd74335959600

CMakeLists.txt

@@ -26,11 +26,10 @@ set(CMAKE_C_EXTENSIONS OFF)
 
 list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake)
 
-# We do not use CMake's BUILD_SHARED_LIBS option.
-option(SECP256K1_BUILD_SHARED "Build shared library." ON)
-option(SECP256K1_BUILD_STATIC "Build static library." ON)
-if(NOT SECP256K1_BUILD_SHARED AND NOT SECP256K1_BUILD_STATIC)
-  message(FATAL_ERROR "At least one of SECP256K1_BUILD_SHARED and SECP256K1_BUILD_STATIC must be enabled.")
+option(BUILD_SHARED_LIBS "Build shared libraries" ON)
+option(SECP256K1_DISABLE_SHARED "Disable shared library. Overrides BUILD_SHARED_LIBS." OFF)
+if(SECP256K1_DISABLE_SHARED)
+  set(BUILD_SHARED_LIBS OFF)
 endif()
 
 option(SECP256K1_ENABLE_MODULE_ECDH "Enable ECDH module." ON)
@@ -225,8 +224,13 @@ message("\n")
 message("secp256k1 configure summary")
 message("===========================")
 message("Build artifacts:")
-message("  shared library ...................... ${SECP256K1_BUILD_SHARED}")
-message("  static library ...................... ${SECP256K1_BUILD_STATIC}")
+if(BUILD_SHARED_LIBS)
+  set(library_type "Shared")
+else()
+  set(library_type "Static")
+endif()
+
+message("  library type ........................ ${library_type}")
 message("Optional modules:")
 message("  ECDH ................................ ${SECP256K1_ENABLE_MODULE_ECDH}")
 message("  ECDSA pubkey recovery ............... ${SECP256K1_ENABLE_MODULE_RECOVERY}")

examples/CMakeLists.txt

@@ -6,15 +6,11 @@ target_compile_options(example INTERFACE
   $<$<C_COMPILER_ID:MSVC>:/wd4005>
 )
 target_link_libraries(example INTERFACE
+  secp256k1
   $<$<PLATFORM_ID:Windows>:bcrypt>
 )
-if(SECP256K1_BUILD_SHARED)
-  target_link_libraries(example INTERFACE secp256k1)
-elseif(SECP256K1_BUILD_STATIC)
-  target_link_libraries(example INTERFACE secp256k1_static)
-  if(MSVC)
-    target_link_options(example INTERFACE /IGNORE:4217)
-  endif()
+if(NOT BUILD_SHARED_LIBS AND MSVC)
+  target_link_options(example INTERFACE /IGNORE:4217)
 endif()
 
 add_executable(ecdsa_example ecdsa.c)

src/CMakeLists.txt

@@ -1,107 +1,79 @@
 # Must be included before CMAKE_INSTALL_INCLUDEDIR is used.
 include(GNUInstallDirs)
-set(${PROJECT_NAME}_installables "")
 
+add_library(secp256k1_precomputed OBJECT EXCLUDE_FROM_ALL
+  precomputed_ecmult.c
+  precomputed_ecmult_gen.c
+)
+
+# Add objects explicitly rather than linking to the object libs to keep them
+# from being exported.
+add_library(secp256k1 secp256k1.c $<TARGET_OBJECTS:secp256k1_precomputed>)
+
+add_library(secp256k1_asm INTERFACE)
 if(SECP256K1_ASM STREQUAL "arm")
-  add_library(common OBJECT
+  add_library(secp256k1_asm_arm OBJECT EXCLUDE_FROM_ALL)
+  target_sources(secp256k1_asm_arm PUBLIC
     asm/field_10x26_arm.s
   )
-  set(common_obj "$<TARGET_OBJECTS:common>")
-else()
-  set(common_obj "")
+  target_sources(secp256k1 PRIVATE $<TARGET_OBJECTS:secp256k1_asm_arm>)
+  target_link_libraries(secp256k1_asm INTERFACE secp256k1_asm_arm)
 endif()
 
-add_library(precomputed OBJECT
-  precomputed_ecmult.c
-  precomputed_ecmult_gen.c
-)
-set(internal_obj "$<TARGET_OBJECTS:precomputed>" "${common_obj}")
+# Define our export symbol only for Win32 and only for shared libs.
+# This matches libtool's usage of DLL_EXPORT
+if(WIN32)
+  set_target_properties(secp256k1 PROPERTIES DEFINE_SYMBOL "DLL_EXPORT")
+endif()
 
-add_library(secp256k1 SHARED EXCLUDE_FROM_ALL
-  secp256k1.c
-  ${internal_obj}
-)
-target_include_directories(secp256k1 INTERFACE
+# Object libs don't know if they're being built for a shared or static lib.
+# Grab the PIC property from secp256k1 which knows.
+get_target_property(use_pic secp256k1 POSITION_INDEPENDENT_CODE)
+set_target_properties(secp256k1_precomputed PROPERTIES POSITION_INDEPENDENT_CODE ${use_pic})
+
+target_include_directories(secp256k1 PUBLIC
   $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
 )
-target_compile_definitions(secp256k1 PRIVATE
-  $<$<PLATFORM_ID:Windows>:DLL_EXPORT>
-)
 set_target_properties(secp256k1 PROPERTIES
   VERSION "${${PROJECT_NAME}_LIB_VERSION_CURRENT}.${${PROJECT_NAME}_LIB_VERSION_AGE}.${${PROJECT_NAME}_LIB_VERSION_REVISION}"
   SOVERSION "${${PROJECT_NAME}_LIB_VERSION_CURRENT}"
 )
-if(SECP256K1_BUILD_SHARED)
-  get_target_property(use_pic secp256k1 POSITION_INDEPENDENT_CODE)
-  set_target_properties(precomputed PROPERTIES POSITION_INDEPENDENT_CODE ${use_pic})
-  set_target_properties(secp256k1 PROPERTIES EXCLUDE_FROM_ALL FALSE)
-  list(APPEND ${PROJECT_NAME}_installables secp256k1)
-endif()
-
-add_library(secp256k1_static STATIC EXCLUDE_FROM_ALL
-  secp256k1.c
-  ${internal_obj}
-)
-target_include_directories(secp256k1_static INTERFACE
-  $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
-)
-if(NOT MSVC)
-  set_target_properties(secp256k1_static PROPERTIES
-    OUTPUT_NAME secp256k1
-  )
-endif()
-if(SECP256K1_BUILD_STATIC)
-  set_target_properties(secp256k1_static PROPERTIES EXCLUDE_FROM_ALL FALSE)
-  list(APPEND ${PROJECT_NAME}_installables secp256k1_static)
-endif()
-
-add_library(binary_interface INTERFACE)
-target_compile_definitions(binary_interface INTERFACE
-  $<$<C_COMPILER_ID:MSVC>:_CRT_SECURE_NO_WARNINGS>
-)
-
-add_library(link_library INTERFACE)
-if(SECP256K1_BUILD_SHARED)
-  target_link_libraries(link_library INTERFACE secp256k1)
-elseif(SECP256K1_BUILD_STATIC)
-  target_link_libraries(link_library INTERFACE secp256k1_static)
-endif()
 
 if(SECP256K1_BUILD_BENCHMARK)
   add_executable(bench bench.c)
-  target_link_libraries(bench binary_interface link_library)
-  add_executable(bench_internal bench_internal.c ${internal_obj})
-  target_link_libraries(bench_internal binary_interface)
-  add_executable(bench_ecmult bench_ecmult.c ${internal_obj})
-  target_link_libraries(bench_ecmult binary_interface)
+  target_link_libraries(bench secp256k1)
+  add_executable(bench_internal bench_internal.c)
+  target_link_libraries(bench_internal secp256k1_precomputed secp256k1_asm)
+  add_executable(bench_ecmult bench_ecmult.c)
+  target_link_libraries(bench_ecmult secp256k1_precomputed secp256k1_asm)
 endif()
 
 if(SECP256K1_BUILD_TESTS)
-  add_executable(noverify_tests tests.c ${internal_obj})
-  target_link_libraries(noverify_tests binary_interface)
+  add_executable(noverify_tests tests.c)
+  target_link_libraries(noverify_tests secp256k1_precomputed secp256k1_asm)
   add_test(noverify_tests noverify_tests)
   if(NOT CMAKE_BUILD_TYPE STREQUAL "Coverage")
-    add_executable(tests tests.c ${internal_obj})
+    add_executable(tests tests.c)
     target_compile_definitions(tests PRIVATE VERIFY)
-    target_link_libraries(tests binary_interface)
+    target_link_libraries(tests secp256k1_precomputed secp256k1_asm)
     add_test(tests tests)
   endif()
 endif()
 
 if(SECP256K1_BUILD_EXHAUSTIVE_TESTS)
-  # Note: do not include $<TARGET_OBJECTS:precomputed> in exhaustive_tests (it uses runtime-generated tables).
-  add_executable(exhaustive_tests tests_exhaustive.c ${common_obj})
+  # Note: do not include secp256k1_precomputed in exhaustive_tests (it uses runtime-generated tables).
+  add_executable(exhaustive_tests tests_exhaustive.c)
+  target_link_libraries(exhaustive_tests secp256k1_asm)
   target_compile_definitions(exhaustive_tests PRIVATE $<$<NOT:$<CONFIG:Coverage>>:VERIFY>)
-  target_link_libraries(exhaustive_tests binary_interface)
   add_test(exhaustive_tests exhaustive_tests)
 endif()
 
 if(SECP256K1_BUILD_CTIME_TESTS)
   add_executable(ctime_tests ctime_tests.c)
-  target_link_libraries(ctime_tests binary_interface link_library)
+  target_link_libraries(ctime_tests secp256k1)
 endif()
 
-install(TARGETS ${${PROJECT_NAME}_installables}
+install(TARGETS secp256k1
   EXPORT ${PROJECT_NAME}-targets
   RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
   LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}