icl-utk-edu / blaspp

BLAS++ is a C++ wrapper around CPU and GPU BLAS (basic linear algebra subroutines), developed as part of the SLATE project.
https://icl.utk.edu/slate/
BSD 3-Clause "New" or "Revised" License
66 stars 23 forks source link

Shared libarary is unversioned #63

Closed mbanck closed 5 months ago

mbanck commented 1 year ago

blaspp builds a shared library (libblaspp.so) by default, but does not version it - this is hazardous for interoperability with other packages.

Please consider versioning your shared library API, or, if your API is unstable, disabling shared library support for the time being.

I am packaging blaspp for Debian/Ubuntu and will only provide a static development library for now.

luszczek commented 1 year ago

Is this based on any particular issue you've noticed in the packaging process? We only see that C++ binaries and libraries are implicitly versioned by the C++ standard library linked in at build time. And this is so without even mentioning the fact that one of the Debian/Ubunty-provided BLAS implementations would provide further binary tagging services. Also, if a user-provided libblaspp.so of unknown origin would appear on a Linux system then it would likely be in a non-default location. So such a unversioned library would have to be pointed to directly by one of the standard mechanism for locating dynamically-linked objects such as environment variable or explicit rpath manipulation in ELF format. I could imagine that you could point us towards Debian/Ubuntu policy requiring DL versioning system-wide. We are usually not involved in the Linux distribution development and packaging processes and we would benefit from extra details.

topazus commented 10 months ago

Adding soname when building shared library is also suggested on Fedora.

Ref: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_shared_libraries

badshah400 commented 6 months ago

This is mandatory for shared libraries on openSUSE, and I would have liked to send you a patch that does this. Unfortunately, the YYYY-MM-DD format of versioning for a library makes it difficult to understand what API consistency guarantees are provided during version bumps. Will a bump from 2023 to 2024 in the major version be backward incompatible for instance? Does the change in the MM part of the version imply API consistency?

There is good reason to instead follow semantic versioning for libraries, as described here: https://semver.org/. See also https://cmake.org/cmake/help/latest/prop_tgt/SOVERSION.html.

I hope that provides a bit more clarity on why this is desired (or enforced) by distributions.

badshah400 commented 6 months ago

FWIW, the following patch assumes the major version ensures API/ABI stability and enforces the ABI and SOVERSION accordingly:

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

Index: blaspp-2023.11.05/CMakeLists.txt
===================================================================
--- blaspp-2023.11.05.orig/CMakeLists.txt
+++ blaspp-2023.11.05/CMakeLists.txt
@@ -241,6 +241,8 @@ add_library(
     src/onemkl_wrappers.cc
 )

+set_target_properties(blaspp PROPERTIES VERSION ${PROJECT_VERSION} SOVERSION ${PROJECT_VERSION_MAJOR})
+
 #-------------------------------------------------------------------------------
 # CUDA support.
 message( "" )
mgates3 commented 6 months ago

Thanks for the comments. Yes, I understand the reasoning for semantic versioning. Given our limited resources, it is difficult to track ABI compatibility from version to version. I would not expect releases to be ABI compatible, since deprecated routines are removed on occasion, and structure's internals change. We find that calendar versioning has certain benefits, as well. We may use a separate semantic versioning for the soversion, if we can develop a simple means to track ABI compatibility.

badshah400 commented 6 months ago

Appreciate the comment. Just would like to add that, you may want to consider an SOVERSION of 0 that provides no API/ABI guarantees. May libraries do this until they are confident of preserving compatibility at which point they bump it to 1.