-
Notifications
You must be signed in to change notification settings - Fork 566
CONFIGURE: added a configure option to add SONAME suffix #11483
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
base: master
Are you sure you want to change the base?
Changes from 4 commits
61f87c0
322570f
bae9f33
0350617
514e20b
40f47f7
9b24baf
c38c388
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,40 @@ AC_SUBST(EXTRA_VERSION) | |
| AC_SUBST(SCM_VERSION) | ||
| AC_SUBST(SOVERSION) | ||
|
|
||
| UCX_LT_RELEASE= | ||
| ucx_soname_suffix_summary="<disabled>" | ||
| ucx_module_deepbind_summary="no" | ||
| AC_ARG_WITH([soname-suffix], | ||
| AS_HELP_STRING([--with-soname-suffix=SUFFIX], | ||
| [Append SUFFIX to UCX installed shared library names and SONAMEs. Disabled by default. [default=NO]]), | ||
| [], [with_soname_suffix=no]) | ||
| AS_IF([test "x$with_soname_suffix" != xno], | ||
| [AS_IF([test "x$with_soname_suffix" = xyes], | ||
| [AC_MSG_ERROR([--with-soname-suffix requires an explicit suffix value])]) | ||
| case "$with_soname_suffix" in | ||
| @<:@A-Za-z0-9@:>@*) ;; | ||
| *) AC_MSG_ERROR([--with-soname-suffix must start with a letter or digit]) ;; | ||
| esac | ||
| case "$with_soname_suffix" in | ||
| *@<:@!A-Za-z0-9_-@:>@*) | ||
| AC_MSG_ERROR([--with-soname-suffix must contain only letters, digits, underscores, and dashes]) | ||
| ;; | ||
| esac | ||
| UCX_LT_RELEASE="-release $with_soname_suffix" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ℹ️ INFO only the four core libs (ucm/ucp/ucs/uct) get |
||
| AC_DEFINE_UNQUOTED([UCX_MODULE_FILE_SUFFIX], | ||
| ["-$with_soname_suffix"], | ||
| [Suffix appended to private UCX module file names]) | ||
| ucx_soname_suffix_summary="$with_soname_suffix"]) | ||
| AC_SUBST([UCX_LT_RELEASE]) | ||
|
|
||
| AC_ARG_ENABLE([module-deepbind], | ||
| AS_HELP_STRING([--enable-module-deepbind], | ||
| [Load UCX modules with RTLD_DEEPBIND. Intended for private UCX bundles. Disabled by default. [default=NO]]), | ||
| [], [enable_module_deepbind=no]) | ||
| AS_IF([test "x$enable_module_deepbind" = xyes], | ||
| [AS_IF([test "x$with_soname_suffix" = xno], | ||
| [AC_MSG_ERROR([--enable-module-deepbind requires --with-soname-suffix])])]) | ||
|
|
||
| AC_PROG_CC | ||
| AC_PROG_CXX | ||
| AC_OPENMP | ||
|
|
@@ -87,6 +121,14 @@ AC_FUNC_STRERROR_R | |
|
|
||
| AC_PATH_TOOL([PKG_CONFIG], [pkg-config], [pkg-config]) | ||
|
|
||
| AS_IF([test "x$enable_module_deepbind" = xyes], | ||
| [AC_CHECK_DECLS([RTLD_DEEPBIND], | ||
| [AC_DEFINE([UCX_MODULE_DLOPEN_DEEPBIND], [1], | ||
| [Load UCX modules with RTLD_DEEPBIND]) | ||
| ucx_module_deepbind_summary="yes"], | ||
| [AC_MSG_ERROR([--enable-module-deepbind requires RTLD_DEEPBIND support from <dlfcn.h>])], | ||
| [[#include <dlfcn.h>]])]) | ||
|
roiedanino marked this conversation as resolved.
|
||
|
|
||
|
|
||
| # | ||
| # Define SHARED_LIB preprocessor macro when building a shared library | ||
|
|
@@ -434,6 +476,8 @@ AC_MSG_NOTICE([ Multi-thread: ${mt_enable}]) | |
| AC_MSG_NOTICE([ MPI tests: ${mpi_enable}]) | ||
| AC_MSG_NOTICE([ VFS support: ${vfs_enable}]) | ||
| AC_MSG_NOTICE([ Devel headers: ${enable_devel_headers}]) | ||
| AC_MSG_NOTICE([ SONAME suffix: ${ucx_soname_suffix_summary}]) | ||
| AC_MSG_NOTICE([ Module deepbind: ${ucx_module_deepbind_summary}]) | ||
| AC_MSG_NOTICE([io_demo CUDA support: ${with_iodemo_cuda}]) | ||
| AC_MSG_NOTICE([ Bindings: <$(echo ${build_bindings}|tr ':' ' ') >]) | ||
| AC_MSG_NOTICE([ UCS modules: <$(echo ${ucs_modules}|tr ':' ' ') >]) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ libucs_la_CPPFLAGS = $(BASE_CPPFLAGS) $(BFD_CPPFLAGS) \ | |
| -DUCX_MODULE_DIR=\"$(moduledir)\" \ | ||
| -DUCX_CONFIG_DIR=\"$(ucx_config_dir)\" | ||
| libucs_la_CFLAGS = $(BASE_CFLAGS) $(LT_CFLAGS) | ||
| libucs_la_LDFLAGS = -ldl $(BFD_LDFLAGS) -version-info $(SOVERSION) | ||
| libucs_la_LDFLAGS = -ldl $(BFD_LDFLAGS) -version-info $(SOVERSION) $(UCX_LT_RELEASE) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] [important] libucs_signal is left unsuffixed The new suffix is applied to libucm/libucs/libuct/libucp, but libucs_signal is also an installed versioned UCX shared library. If the goal is avoiding collisions with already-loaded UCX DSOs, leaving this SONAME as |
||
| libucs_ladir = $(includedir)/ucs | ||
| libucs_la_LIBADD = $(LIBM) $(top_builddir)/src/ucm/libucm.la $(BFD_LIBS) | ||
|
|
||
|
|
@@ -130,6 +130,7 @@ noinst_HEADERS = \ | |
| sys/compiler.h \ | ||
| sys/lib.h \ | ||
| sys/module.h \ | ||
| sys/module_int.h \ | ||
| sys/sys.h \ | ||
| sys/iovec.h \ | ||
| sys/iovec.inl \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| #endif | ||
|
|
||
| #include "module.h" | ||
| #include "module_int.h" | ||
|
|
||
| #include <ucs/sys/preprocessor.h> | ||
| #include <ucs/datastruct/string_buffer.h> | ||
|
|
@@ -245,6 +246,9 @@ static int ucs_module_flags_to_dlopen_mode(unsigned flags) | |
| } else { | ||
| mode |= RTLD_LOCAL; | ||
| } | ||
| #if defined(UCX_MODULE_DLOPEN_DEEPBIND) && defined(RTLD_DEEPBIND) | ||
| mode |= RTLD_DEEPBIND; | ||
| #endif | ||
|
|
||
| return mode; | ||
| } | ||
|
|
@@ -324,6 +328,7 @@ static void ucs_module_load_from_dir(const char *dir, const char *framework, | |
| } | ||
|
|
||
| ucs_module_filename_to_base(entry->d_name, base, sizeof(base)); | ||
| ucs_module_normalize_base(base); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| if (strchr(base + prefix_len, '_') != NULL) { | ||
| ucs_module_debug("module name contains '_': %s, skipping", base + prefix_len); | ||
| continue; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| /** | ||
| * Copyright (c) NVIDIA CORPORATION & AFFILIATES, 2026. ALL RIGHTS RESERVED. | ||
| * | ||
| * See file LICENSE for terms. | ||
| */ | ||
|
|
||
| #ifndef UCS_MODULE_INT_H_ | ||
| #define UCS_MODULE_INT_H_ | ||
|
|
||
| #ifdef HAVE_CONFIG_H | ||
| # include "config.h" | ||
| #endif | ||
|
|
||
| #include <string.h> | ||
|
|
||
|
|
||
| static inline void ucs_module_normalize_base(char *base) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ℹ️ INFO minor: a dedicated header for a single 8-line |
||
| { | ||
| #ifdef UCX_MODULE_FILE_SUFFIX | ||
| size_t suffix_len = strlen(UCX_MODULE_FILE_SUFFIX); | ||
| size_t base_len = strlen(base); | ||
|
|
||
| /* Expected input after stripping .so is lib<framework>_<module>-<suffix>. */ | ||
| if ((base_len > suffix_len) && | ||
| !strcmp(base + base_len - suffix_len, UCX_MODULE_FILE_SUFFIX)) { | ||
| base[base_len - suffix_len] = '\0'; | ||
| } | ||
| #endif | ||
| } | ||
|
|
||
|
|
||
| #endif | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨 BLOCKER
libtool's
-releasechanges the installed library filename (e.g.libucp-SUFFIX.so.0), not just the SONAME. With this set,ucx.pc(which emits-lucp) andcmake/ucx-targets.cmake.in(which hardcodeslibucp.so/libucs.so/libuct.so) will fail to locate the libs for downstream consumers. Either also template these files with the suffix, or use a mechanism that only alters the SONAME (e.g.,-Wl,-soname=...) if the intent really is SONAME-only as the option name suggests.