Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add static build config. #510

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 11 additions & 5 deletions CMakeLists.txt
@@ -1,6 +1,5 @@

# Options
option(SLEEF_BUILD_SHARED_LIBS "Build sleef as static library" ON)
option(SLEEF_BUILD_STATIC_TEST_BINS "Build statically linked test executables" OFF)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior of option and set are controlled by policies. You must set cmake_minimum_required as the first line of your top-level program to have any backwards compatibility guarantees at all.

Ideally project() would be the second line since so many other commands depend on the internal state it configures.

Suggested change
option(SLEEF_BUILD_STATIC_TEST_BINS "Build statically linked test executables" OFF)
cmake_minimum_required(VERSION 3.18)
project(SLEEF LANGUAGES C VERSION 3.6.0)
# TODO: rename uses of this variable to use the standard one
set(SLEEF_VERSION_PATCHLEVEL ${SLEEF_VERSION_PATCH})
set(SLEEF_SOVERSION ${SLEEF_VERSION_MAJOR})
option(SLEEF_BUILD_STATIC_TEST_BINS "Build statically linked test executables" OFF)

Then, after your options, add the following lines:

if (SLEEF_ENABLE_CXX)
  enable_language(CXX)
endif ()
if (SLEEF_ENABLE_CUDA)
  enable_language(CUDA)
endif ()

No need to maintain that LANGLIST anymore. Just enable the requested languages.

Then, below, delete these lines:

cmake_minimum_required(VERSION 3.18)
if(${CMAKE_VERSION} VERSION_GREATER "3.14.99")
  cmake_policy(SET CMP0091 NEW)
endif()

CMP0091 is enabled in CMake 3.15+ so the cmake_policy line is a noop and so the whole if () block can be removed.

Also delete these lines:

set(SLEEF_VERSION_MAJOR 3)
set(SLEEF_VERSION_MINOR 6)
set(SLEEF_VERSION_PATCHLEVEL 0)
set(SLEEF_VERSION ${SLEEF_VERSION_MAJOR}.${SLEEF_VERSION_MINOR}.${SLEEF_VERSION_PATCHLEVEL})
set(SLEEF_SOVERSION ${SLEEF_VERSION_MAJOR})
set(LANGLIST C)
if (SLEEF_ENABLE_CUDA)
  set(LANGLIST ${LANGLIST} CUDA)
endif()
if (SLEEF_ENABLE_CXX)
  set(LANGLIST ${LANGLIST} CXX)
endif()

option(SLEEF_ENABLE_LTO "Enable LTO on GCC or ThinLTO on clang" OFF)
option(SLEEF_BUILD_LIBM "libsleef will be built." ON)
Expand Down Expand Up @@ -29,6 +28,13 @@ option(SLEEF_DISABLE_SSL "Disable testing with the SSL library" OFF)
option(SLEEF_ENABLE_CUDA "Enable CUDA" OFF)
option(SLEEF_ENABLE_CXX "Enable C++" OFF)

set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
if(NOT DEFINED ${SLEEF_BUILD_SHARED_LIBS})
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS})
else()
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${SLEEF_BUILD_SHARED_LIBS})
endif()

Comment on lines +31 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be done with options instead? For instance,

option(BUILD_SHARED_LIBS "Build sleef as shared library" ON)
option(SLEEF_BUILD_SHARED_LIBS "Build sleef as shared library" ${BUILD_SHARED_LIBS})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blapie -- These are not equivalent because this defines SLEEF_BUILD_SHARED_LIBS as a cache variable when no variable by that name (normal or cache) exists. So then changing BUILD_SHARED_LIBS only works as expected on the first configuration.

My blog post suggests the following:

Suggested change
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
if(NOT DEFINED ${SLEEF_BUILD_SHARED_LIBS})
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS})
else()
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${SLEEF_BUILD_SHARED_LIBS})
endif()
if (DEFINED SLEEF_BUILD_SHARED_LIBS)
set(BUILD_SHARED_LIBS ${SLEEF_BUILD_SHARED_LIBS})
endif ()

Then you can just look at the truthiness of BUILD_SHARED_LIBS down the line. If you care about documenting this variable in the CMake CLI/GUI, then this approach works:

Suggested change
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
if(NOT DEFINED ${SLEEF_BUILD_SHARED_LIBS})
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS})
else()
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${SLEEF_BUILD_SHARED_LIBS})
endif()
set(SLEEF_BUILD_SHARED_LIBS "undefined"
CACHE BOOL "Override BUILD_SHARED_LIBS while configuring SLEEF")
if (NOT SLEEF_BUILD_SHARED_LIBS STREQUAL "undefined")
set(BUILD_SHARED_LIBS ${SLEEF_BUILD_SHARED_LIBS})
endif ()

# Function used to generate safe command arguments for add_custom_command
function(command_arguments PROPNAME)
set(quoted_args "")
Expand Down Expand Up @@ -157,9 +163,9 @@ separate build directory. Note: Please remove autogenerated file \
`CMakeCache.txt` and directory `CMakeFiles` in the current directory.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everywhere you should just use SLEEF_SOURCE_DIR and SLEEF_BINARY_DIR since those are the standard and expected names. The project command defines them and the VERSION variables.

https://cmake.org/cmake/help/v3.18/command/project.html#project

endif()

if(SLEEF_ENABLE_LTO AND SLEEF_BUILD_SHARED_LIBS)
message(FATAL_ERROR "SLEEF_ENABLE_LTO and SLEEF_BUILD_SHARED_LIBS cannot be specified at the same time")
endif(SLEEF_ENABLE_LTO AND SLEEF_BUILD_SHARED_LIBS)
if(SLEEF_ENABLE_LTO AND _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
message(FATAL_ERROR "SLEEF_ENABLE_LTO and _INTERNAL_SLEEF_BUILD_SHARED_LIBS cannot be specified at the same time")
endif(SLEEF_ENABLE_LTO AND _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
Comment on lines +166 to +168
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With my other suggestions:

Suggested change
if(SLEEF_ENABLE_LTO AND _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
message(FATAL_ERROR "SLEEF_ENABLE_LTO and _INTERNAL_SLEEF_BUILD_SHARED_LIBS cannot be specified at the same time")
endif(SLEEF_ENABLE_LTO AND _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
if (SLEEF_ENABLE_LTO AND BUILD_SHARED_LIBS)
message(FATAL_ERROR "SLEEF_ENABLE_LTO and (SLEEF_)BUILD_SHARED_LIBS cannot be enabled at the same time")
endif ()

Note that duplicating the condition in the endif hasn't been a requirement since at least 2.8.x but it might be as far back as 2.4.x. Definitely not needed in 3.18, which is your minimum.


if(SLEEF_ENABLE_LTO)
cmake_policy(SET CMP0069 NEW)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cmake_policy(SET CMP0069 NEW)

That policy is already enabled by cmake_minimum_required(VERSION 3.18). It's enabled for 3.9+.

Expand Down Expand Up @@ -305,7 +311,7 @@ if(SLEEF_SHOW_CONFIG)
message(" Native build dir: ${NATIVE_BUILD_DIR}")
endif(CMAKE_CROSSCOMPILING)
message(STATUS "Using option `${SLEEF_C_FLAGS}` to compile libsleef")
message(STATUS "Building shared libs : " ${SLEEF_BUILD_SHARED_LIBS})
message(STATUS "Building shared libs : " ${_INTERNAL_SLEEF_BUILD_SHARED_LIBS})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
message(STATUS "Building shared libs : " ${_INTERNAL_SLEEF_BUILD_SHARED_LIBS})
message(STATUS "Building shared libs : " ${BUILD_SHARED_LIBS})

message(STATUS "Building static test bins: " ${SLEEF_BUILD_STATIC_TEST_BINS})
message(STATUS "MPFR : " ${LIB_MPFR})
if (MPFR_INCLUDE_DIR)
Expand Down
4 changes: 2 additions & 2 deletions Configure.cmake
Expand Up @@ -7,7 +7,7 @@ include(CheckLanguage)

if (SLEEF_BUILD_STATIC_TEST_BINS)
set(CMAKE_FIND_LIBRARY_SUFFIXES ".a")
set(SLEEF_BUILD_SHARED_LIBS OFF)
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS OFF)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS OFF)
set(BUILD_SHARED_LIBS OFF)

set(CMAKE_EXE_LINKER_FLAGS "-static")
endif()

Expand Down Expand Up @@ -834,7 +834,7 @@ endif()

# Set common definitions

if (NOT SLEEF_BUILD_SHARED_LIBS)
if (NOT _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (NOT _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
if (NOT BUILD_SHARED_LIBS)

set(COMMON_TARGET_DEFINITIONS SLEEF_STATIC_LIBS=1)
set(SLEEF_STATIC_LIBS 1)
endif()
Expand Down
2 changes: 1 addition & 1 deletion src/common/CMakeLists.txt
Expand Up @@ -4,7 +4,7 @@ set(COMMON_TARGET_PROPERTIES
C_STANDARD 99 # -std=gnu99
)

if (SLEEF_BUILD_SHARED_LIBS)
if (_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
list(APPEND COMMON_TARGET_PROPERTIES POSITION_INDEPENDENT_CODE ON) # -fPIC
endif()

Expand Down
4 changes: 2 additions & 2 deletions src/dft/CMakeLists.txt
Expand Up @@ -224,7 +224,7 @@ set(COMMON_TARGET_PROPERTIES
C_STANDARD 99 # -std=gnu99
)

if (SLEEF_BUILD_SHARED_LIBS)
if (_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
list(APPEND COMMON_TARGET_PROPERTIES POSITION_INDEPENDENT_CODE ON) # -fPIC
endif()

Expand Down Expand Up @@ -373,7 +373,7 @@ foreach(T ${LIST_SUPPORTED_FPTYPE})
endforeach()

# Target libdft
if(SLEEF_BUILD_SHARED_LIBS)
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(${TARGET_LIBDFT} SHARED $<TARGET_OBJECTS:dftcommon_obj> $<TARGET_OBJECTS:${TARGET_LIBARRAYMAP_OBJ}>)
else()
add_library(${TARGET_LIBDFT} STATIC $<TARGET_OBJECTS:dftcommon_obj> $<TARGET_OBJECTS:${TARGET_LIBARRAYMAP_OBJ}>)
Expand Down
8 changes: 4 additions & 4 deletions src/libm/CMakeLists.txt
Expand Up @@ -348,7 +348,7 @@ set(COMMON_TARGET_PROPERTIES
C_STANDARD 99 # -std=gnu99
)

if (SLEEF_BUILD_SHARED_LIBS)
if (_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
list(APPEND COMMON_TARGET_PROPERTIES POSITION_INDEPENDENT_CODE ON) # -fPIC
endif()

Expand All @@ -359,7 +359,7 @@ endif()
# Original sleef sources
set(STANDARD_SOURCES rempitab.c)

if(SLEEF_BUILD_SHARED_LIBS)
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(${TARGET_LIBSLEEF} SHARED ${STANDARD_SOURCES})
else()
add_library(${TARGET_LIBSLEEF} STATIC ${STANDARD_SOURCES})
Expand Down Expand Up @@ -889,7 +889,7 @@ if(ENABLE_GNUABI)
endforeach()

# Create library
if(SLEEF_BUILD_SHARED_LIBS)
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(${TARGET_LIBSLEEFGNUABI} SHARED ${TARGET_LIBSLEEFGNUABI_OBJECTS} rempitab.c)
else()
add_library(${TARGET_LIBSLEEFGNUABI} STATIC ${TARGET_LIBSLEEFGNUABI_OBJECTS} rempitab.c)
Expand Down Expand Up @@ -982,7 +982,7 @@ endif()
# --------------------------------------------------------------------
# Build scalar-only library from sleefdp.c and sleefsp.c
if(SLEEF_BUILD_SCALAR_LIB)
if(SLEEF_BUILD_SHARED_LIBS)
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(sleefscalar SHARED sleefdp.c sleefsp.c rempitab.c)
else()
add_library(sleefscalar STATIC sleefdp.c sleefsp.c rempitab.c)
Expand Down
4 changes: 2 additions & 2 deletions src/quad/CMakeLists.txt
Expand Up @@ -112,7 +112,7 @@ if(COMPILER_SUPPORTS_BUILTIN_MATH)
list(APPEND COMMON_TARGET_DEFINITIONS ENABLE_BUILTIN_MATH=1)
endif()

if (SLEEF_BUILD_SHARED_LIBS)
if (_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
list(APPEND COMMON_TARGET_PROPERTIES POSITION_INDEPENDENT_CODE ON)
endif()

Expand Down Expand Up @@ -214,7 +214,7 @@ foreach(SIMD ${SLEEF_SUPPORTED_QUAD_EXTENSIONS})
endif()
endforeach()

if(SLEEF_BUILD_SHARED_LIBS)
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(sleefquad SHARED rempitabqp.c ${SLEEFQUAD_OBJECTS})
else()
add_library(sleefquad STATIC rempitabqp.c ${SLEEFQUAD_OBJECTS})
Expand Down