RenderKit / oidn

Intel® Open Image Denoise library
https://www.openimagedenoise.org/
Apache License 2.0
1.73k stars 160 forks source link

missing soversion for the sub libraries like _core and _device_cpu #213

Open darix opened 2 months ago

darix commented 2 months ago

just wondering if that is done on purpose or if there is a patch missing like

diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt
index 821f8e3..75891bc 100644
--- a/core/CMakeLists.txt
+++ b/core/CMakeLists.txt
@@ -89,6 +89,7 @@ endif()

 add_library(OpenImageDenoise_core ${OIDN_CORE_LIB_TYPE} ${OIDN_CORE_SOURCES} ${OIDN_RESOURCE_FILE})
 set_property(TARGET OpenImageDenoise_core PROPERTY VERSION ${PROJECT_VERSION})
+set_property(TARGET OpenImageDenoise_core PROPERTY SOVERSION ${PROJECT_VERSION_MAJOR})

 target_link_libraries(OpenImageDenoise_core
   PUBLIC
diff --git a/devices/cpu/CMakeLists.txt b/devices/cpu/CMakeLists.txt
index ba30e28..7b1935b 100644
--- a/devices/cpu/CMakeLists.txt
+++ b/devices/cpu/CMakeLists.txt
@@ -122,6 +122,7 @@ endif()

 add_library(OpenImageDenoise_device_cpu ${OIDN_LIB_TYPE} ${OIDN_CPU_SOURCES} ${OIDN_RESOURCE_FILE})
 set_property(TARGET OpenImageDenoise_device_cpu PROPERTY VERSION ${PROJECT_VERSION})
+set_property(TARGET OpenImageDenoise_device_cpu PROPERTY SOVERSION ${PROJECT_VERSION_MAJOR})

 if(OIDN_DNNL)
   target_compile_definitions(OpenImageDenoise_device_cpu PRIVATE OIDN_DNNL)
atafra commented 2 months ago

It's done on purpose.

StefanBruens commented 2 months ago

As SOVERSION defaults to VERSION, the libraries are already versioned.

If any external user only ever (directly) links to libOpenImageDenoise.so.2, and the APIs/ABIs of the core and device libraries are not exported, this is a reasonable setup.

(If this is the case, it may be a good idea to still set the SOVERSION = ${PROJECT_VERSION} explicitly, and add a corresponding comment noting the core library is not exported and does not fall under stable ABI guarantees).

darix commented 2 months ago

@StefanBruens it adds a bit of fun with our shared library policy as it then wants us to use 2_2_2 as suffix for the shared library package. my devel package now has the above patch in to make them all .so.2 so rpmlint shuts up. if we remove the patch we need to decide how to handle those other libraries. subpackages with hard equal requires or rpmlintrc and leaving them in one package.

atafra commented 2 months ago

The _core and _device_* libraries aren't exported and don't have a stable ABI. The version of these must match the version of the main library.

darix commented 2 months ago

@StefanBruens package here: https://build.opensuse.org/package/show/home:darix:branches:graphics/OpenImageDenoise

darix commented 2 months ago

the device_cpu library is later loaded via dlopen()? i do not see it linked.

StefanBruens commented 2 months ago

@StefanBruens it adds a bit of fun with our shared library policy as it then wants us to use 2_2_2 as suffix for the shared library package. my devel package now has the above patch in to make them all .so.2 so rpmlint shuts up. if we remove the patch we need to decide how to handle those other libraries. subpackages with hard equal requires or rpmlintrc and leaving them in one package.

Just move them to subpackages - according to the build log, libOpenImageDenoise.so.2 links to libOpenImageDenoise_core.so.2.2.2 at build time. I.e. the subpackage dependency should be picked up by the automatic dependency generator without any further fiddling.