-
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 2 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,36 @@ 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 shared library and module 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])]) | ||
| AS_IF([printf '%s\n' "$with_soname_suffix" | grep -Eq '^@<:@A-Za-z0-9@:>@@<:@A-Za-z0-9_.-@:>@*$'], | ||
|
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. the regex |
||
| [], | ||
| [AC_MSG_ERROR([--with-soname-suffix must contain only letters, digits, dots, underscores, and dashes])]) | ||
| 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. 🚨 BLOCKER libtool's 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_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. minor: the macro name |
||
| ["-$with_soname_suffix"], | ||
| [Suffix appended to private UCX module library names]) | ||
| ucx_soname_suffix_summary="$with_soname_suffix"]) | ||
| AC_SUBST([UCX_LT_RELEASE]) | ||
| UCT_MODULE_LDFLAGS='$(UCX_LT_RELEASE)' | ||
|
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.
|
||
| AC_SUBST([UCT_MODULE_LDFLAGS]) | ||
|
|
||
| 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 +117,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 +472,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) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -245,6 +245,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; | ||
| } | ||
|
|
@@ -292,6 +295,19 @@ ucs_module_filename_to_base(const char *filename, char *base, size_t base_max) | |
| base[base_len] = '\0'; | ||
| } | ||
|
|
||
| static void ucs_module_normalize_base(char *base) | ||
| { | ||
| #ifdef UCX_MODULE_SONAME_SUFFIX | ||
| size_t suffix_len = strlen(UCX_MODULE_SONAME_SUFFIX); | ||
| size_t base_len = strlen(base); | ||
|
|
||
| if ((base_len > suffix_len) && | ||
| !strcmp(base + base_len - suffix_len, UCX_MODULE_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. minor: this only strips the suffix when it appears at the very end of |
||
| base[base_len - suffix_len] = '\0'; | ||
| } | ||
| #endif | ||
| } | ||
|
|
||
| static void ucs_module_load_from_dir(const char *dir, const char *framework, | ||
| int mode, ucs_string_set_t *loaded_set) | ||
| { | ||
|
|
@@ -324,6 +340,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; | ||
|
|
||
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.
[P2] [important] Dotted suffixes break module loading
The option accepts dots, but libtool
-release 1.22produces names likelibucs-1.22.so.0; UCX's module loader derivesmodule_extfrom the first dot in the loadedlibucsbasename, so it would look for module files ending in.22.so.0instead of.so.0. That means accepted values such as--with-soname-suffix=1.22can prevent optional UCT/UCM modules from loading. Either reject dots here or fix the loader to derive the real shared-library extension, and add a small configure/runtime check for a dotted suffix.