libtom / libtommath

LibTomMath is a free open source portable number theoretic multiple-precision integer library written entirely in C.
https://www.libtom.net
Other
656 stars 194 forks source link

Release 1.3.0 #574

Closed sjaeckel closed 8 months ago

sjaeckel commented 8 months ago

@dod38fr @mrc0mmand @gahr @DimStar77 (or maybe @ana ?) @dfandrich @antonio-rojas @millak

Can you please check whether this release would be fine? I've created a tag v1.3.0-rc1

Please use https://github.com/libtom/libtommath/discussions/575 for general questions and this PR for problems with the build.

mrc0mmand commented 8 months ago

Just pushed this to Fedora Rawhide (https://bodhi.fedoraproject.org/updates/FEDORA-2024-37b74bed11), let's see what's going to break :)

dod38fr commented 8 months ago

I've built rakudo 2022.12 on top of libtommath 1.30.0-rc1 without problem, so it looks good.

gahr commented 8 months ago

Hi, I am trying this out using CMake (previously we were using makefile.shared on FreeBSD). I have two issues:

  1. the manpage is not there anymore
  2. it doesn't seem to be possible to build both static and shared libs in one go
sjaeckel commented 8 months ago

Hi, I am trying this out using CMake (previously we were using makefile.shared on FreeBSD). I have two issues:

Oh, that comment was right in time, I was just finishing the release ;)

Indeed! Are you fine with me simply removing that step of trying to install the manpage? The manpage is also written for the current develop branch, so it doesn't really make sense to install it.

  1. it doesn't seem to be possible to build both static and shared libs in one go

That's true, you have to build it twice, once with -DBUILD_SHARED_LIBS=Off for the static library and once with -DBUILD_SHARED_LIBS=On for the shared one ... I've been told "that's the way how CMake do"^TM ... also I've seen proposals that enable the old behavior, but I didn't like them since they seem too convoluted and I can live with the two separate build steps. Is that a problem for you?

gahr commented 8 months ago

Hi, I am trying this out using CMake (previously we were using makefile.shared on FreeBSD). I have two issues:

Oh, that comment was right in time, I was just finishing the release ;)

Yup - so sorry, it took me a while to get to this!

Indeed! Are you fine with me simply removing the manpage?

Sure, that's fine.

  1. it doesn't seem to be possible to build both static and shared libs in one go

That's true, you have to build it twice, once with -DBUILD_SHARED_LIBS=Off for the static library and once with -DBUILD_SHARED_LIBS=On for the shared one ... I've been told "that's the way how CMake do"^TM ... also I've seen proposals that enable that behavior, but I didn't like them since they seem too convoluted and I can live with the two separate build steps. Is that a problem for you?

CMake supports "object" libraries, where the objects are compiled and can later be used to generate both static and shared libraries. Here's a patch that does that. I would suggest to remove the option to build shared libraries and always produce both flavours :)

--- CMakeLists.txt.orig 2024-03-27 08:40:34.650661000 +0000
+++ CMakeLists.txt      2024-03-27 08:41:25.371702000 +0000
@@ -103,9 +103,14 @@
 # library target
 #-----------------------------------------------------------------------------
 add_library(${PROJECT_NAME}
+    OBJECT
     ${SOURCES}
     ${HEADERS}
 )
+add_library(${PROJECT_NAME}_shared SHARED $<TARGET_OBJECTS:${PROJECT_NAME}>)
+add_library(${PROJECT_NAME}_static STATIC $<TARGET_OBJECTS:${PROJECT_NAME}>)
+set_target_properties(${PROJECT_NAME}_shared PROPERTIES OUTPUT_NAME ${PROJECT_NAME})
+set_target_properties(${PROJECT_NAME}_static PROPERTIES OUTPUT_NAME ${PROJECT_NAME})

 target_include_directories(${PROJECT_NAME} PUBLIC
     $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
@@ -130,6 +135,7 @@
     VERSION ${PROJECT_VERSION}
     SOVERSION ${PROJECT_VERSION_MAJOR}
     PUBLIC_HEADER "${PUBLIC_HEADERS}"
+    POSITION_INDEPENDENT_CODE True
 )

 option(COMPILE_LTO "Build with LTO enabled")
@@ -159,7 +165,7 @@
 set(PROJECT_CONFIG_FILE "${PROJECT_NAME}-config.cmake")
 set(TARGETS_EXPORT_NAME "${PROJECT_NAME}Targets")

-install(TARGETS ${PROJECT_NAME}
+install(TARGETS ${PROJECT_NAME}_shared ${PROJECT_NAME}_static
     EXPORT ${TARGETS_EXPORT_NAME}
     LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
     ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT Libraries
gahr commented 8 months ago

Alright, let me do it better: the SOVERSION needs to be a property of the shared lib, and the PUBLIC_HEADER needs to be on libraries proper:

--- CMakeLists.txt.orig 2024-03-27 09:23:29 UTC                                                                                                                                                 [23/1822]
+++ CMakeLists.txt
@@ -103,9 +103,14 @@ add_library(${PROJECT_NAME}
 # library target                                                                                   
 #-----------------------------------------------------------------------------
 add_library(${PROJECT_NAME}                                                                        
+    OBJECT
     ${SOURCES}                 
     ${HEADERS}                                                                                     
 )                                
+add_library(${PROJECT_NAME}_shared SHARED $<TARGET_OBJECTS:${PROJECT_NAME}>)
+add_library(${PROJECT_NAME}_static STATIC $<TARGET_OBJECTS:${PROJECT_NAME}>)
+set_target_properties(${PROJECT_NAME}_shared PROPERTIES OUTPUT_NAME tommath) 
+set_target_properties(${PROJECT_NAME}_static PROPERTIES OUTPUT_NAME tommath)

 target_include_directories(${PROJECT_NAME} PUBLIC                  
     $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
@@ -126,9 +131,16 @@ set_target_properties(${PROJECT_NAME} PROPERTIES                                                                                                                                    
 endif()                                                                                                                                                                                                 

 set_target_properties(${PROJECT_NAME} PROPERTIES                                                                                                                                                        
-    OUTPUT_NAME tommath
     VERSION ${PROJECT_VERSION}
+    POSITION_INDEPENDENT_CODE True                                                                 
+)                                                                                                  
+             
+set_target_properties(${PROJECT_NAME}_shared PROPERTIES
+    PUBLIC_HEADER "${PUBLIC_HEADERS}"
     SOVERSION ${PROJECT_VERSION_MAJOR}                                                             
+)                                                                                                  
+         
+set_target_properties(${PROJECT_NAME}_static PROPERTIES
     PUBLIC_HEADER "${PUBLIC_HEADERS}"
 )                                                                                                  

@@ -159,7 +171,7 @@ set(TARGETS_EXPORT_NAME "${PROJECT_NAME}Targets")
 set(PROJECT_CONFIG_FILE "${PROJECT_NAME}-config.cmake")
 set(TARGETS_EXPORT_NAME "${PROJECT_NAME}Targets")           

-install(TARGETS ${PROJECT_NAME}
+install(TARGETS ${PROJECT_NAME}_shared ${PROJECT_NAME}_static
     EXPORT ${TARGETS_EXPORT_NAME}
     LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
     ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT Libraries
sjaeckel commented 8 months ago

What were the reasons again why some people don't want PIE code in static libraries?

It can't be size, since the PIE version of the static library is slightly smaller ... runtime cost?

gahr commented 8 months ago

As discussed offline, let's postpone the changes to CMake after 1.3.0 is out.

All looks good on FreeBSD :)