project-gemmi / gemmi

macromolecular crystallography library and utilities
https://project-gemmi.github.io/
Mozilla Public License 2.0
205 stars 42 forks source link

Shared library is unversioned #320

Closed badshah400 closed 4 weeks ago

badshah400 commented 1 month ago

In trying to package RPMs for gemmi for the openSUSE Tumbleweed distro, we found out that the shared library installed by gemmi is unversioned. Versioning of shared libs is recommended to keep track of API compatibility/breakage between versions and to allow parallel installation of multiple so_versions of the same library. Versioning for shared libs is also strictly required by distros like openSUSE, Fedora, and others.

This is a request to add versioning to the gemmi shared library. CMake facilitates this using set_target_properties.

Thanks for developing this useful library.

wojdyr commented 4 weeks ago

Hi, thanks for looking into it. Do you plan to package also any other projects that depend on gemmi? Currently, minor changes in API and ABI happen so frequently that I don't keep track of them. Usually, they don't affect any programs, or only one or two, because they are in new or undocumented parts of the library.

If it helps, I can add a cmake option that sets version of the library to the package version.

badshah400 commented 4 weeks ago

Hi, thanks for looking into it. Do you plan to package also any other projects that depend on gemmi?

This might happen eventually, yes.

Currently, minor changes in API and ABI happen so frequently that I don't keep track of them. Usually, they don't affect any programs, or only one or two, because they are in new or undocumented parts of the library.

I understand. This is covered by the semantic versioning spec, specifically spec-item-4, which recommends using 0 as the so version until API consistency is reached.

Given gemmi versions are currently 0.x.y, it should be ok to do something like this (just a suggestion):

--
 CMakeLists.txt |    1 +
 1 file changed, 1 insertion(+)

Index: gemmi-0.6.6/CMakeLists.txt
===================================================================
--- gemmi-0.6.6.orig/CMakeLists.txt
+++ gemmi-0.6.6/CMakeLists.txt
@@ -215,6 +215,7 @@ target_include_directories(gemmi_cpp PRI

 if (BUILD_SHARED_LIBS)
   target_compile_definitions(gemmi_cpp PUBLIC GEMMI_SHARED)
+  set_target_properties(gemmi_cpp PROPERTIES VERSION ${PROJECT_VERSION} SOVERSION ${PROJECT_VERSION_MAJOR})
 endif()
 target_link_libraries(gemmi_cpp PUBLIC gemmi_headers)

If it helps, I can add a cmake option that sets version of the library to the package version.

Or this too.

Many thanks for your considerate and thoughtful response.

wojdyr commented 4 weeks ago

Good, I added the line that you suggested behind cmake option -D ADD_SOVERSION.

badshah400 commented 4 weeks ago

@wojdyr Perfect, many thanks.