autotools: relax soname pattern for dynamic loading #290
Conversation
SDL makes assumption that each dynamically loaded library must have SONAME matching pattern <libname>.so.<digit>+ hence it discards any file that has two (or more) digits after ".so". in practice however SONAME might be in the form of ie <libname>.so.<major>.<minor>. as a solution keep requirement for dynamically loaded files to be named <libname>.so.* but consider all the possibilities and prefer the shortest one. Fixes: libsdl-org#289 From: libsdl-org/SDL#5901
- project moved to github - patch to fix dynamic loading of libjxl (from libsdl-org/SDL_image#290)
|
Regarding failed macos check I suppose JXL support is expected to be disabled (JXL not listed in dependencies) however Can't really tell what is expectation here. |
|
@madebr, can you look at the CMake macOS build failure? |
|
There is an error in the autotools configure script: https://github.com/libsdl-org/SDL_image/runs/7290582907?check_suite_focus=true#step:16:110 I think the default autotools version on mac can't handle our configure.ac. Perhaps we need to add |
configure
Outdated
| @@ -13426,7 +13477,7 @@ find_lib() | |||
| host_lib_path="$ac_default_prefix/$base_libdir $ac_default_prefix/$base_bindir /usr/$base_libdir /usr/local/$base_libdir" | |||
| fi | |||
| for path in $env_lib_path $gcc_bin_path $gcc_lib_path $host_lib_path; do | |||
| lib=`ls -- $path/$1 2>/dev/null | sed -e '/\.so\..*\./d' -e 's,.*/,,' | sort | tail -1` | |||
| lib=`ls -- $path/$1 2>/dev/null | sed -e 's,.*/,,' | $AWK '{print length() " " $0;}' | sort -n -r | tail -1 | sed 's/^[0-9]\+ //'` | |||
There was a problem hiding this comment.
Does this work on Macos? Because the output of configure complains of an error on the next line.
https://github.com/libsdl-org/SDL_image/runs/7290582907?check_suite_focus=true#step:16:110
There was a problem hiding this comment.
Possibly something to do with awk on macOS, and possibly on other BSDs by extension?
Is this written with gawk in mind? (No, I have no awk skills at all.)
We are compatible at least with ac2.63: just tested myself. |
| lib=[`ls -- $path/$1 2>/dev/null | sed -e 's,.*/,,' | $AWK '{print length() " " $0;}' | sort -n -r | tail -1 | sed -e 's/^[0-9]\+ //'`] | ||
| if test "x$lib" != x; then | ||
| lib=[`ls -- $path/$1 2>/dev/null | sed -e 's,.*/,,' | $AWK '{print length() " " $0;}' | sort -n -r | tail -1 | sed -e 's/^[0-9]+ //'`] | ||
| if test x$lib != x; then |
There was a problem hiding this comment.
This change doesn't seem reflected in generated configure?
There was a problem hiding this comment.
I reverted the change to this line as the error was causes by lib containing a space from bad output of the preceding line.
|
An issue with choosing the shortest one, is that it will prefer development libraries: |
Yeah, this is a problem that we need to fix. I reverted the similar change in SDL for this reason. |
| @@ -284,8 +284,8 @@ DIST_SUBDIRS = . test | |||
| am__DIST_COMMON = $(srcdir)/Makefile.in $(srcdir)/SDL2_image.pc.in \ | |||
| $(srcdir)/SDL2_image.spec.in \ | |||
| $(srcdir)/sdl2_image-config-version.cmake.in \ | |||
| $(srcdir)/sdl2_image-config.cmake.in ar-lib compile \ | |||
There was a problem hiding this comment.
Irrelevant to this PR, but this particular removal is correct: We have no ar-lib in SDL_image source tree. If someone's autogen.sh run is really adding it somehow, then it should be git added. @slouken: hint hint..
First argument of |
|
Primary issue with original PR was use of |
|
Fixed in 79f9995 |
SDL makes assumption that each dynamically loaded library must have
SONAME matching pattern .so.+ hence it discards any file
that has two (or more) digits after ".so". in practice however SONAME
might be in the form of ie .so...
as a solution keep requirement for dynamically loaded files to be named
.so.* but consider all the possibilities and prefer the shortest
one.
Fixes: #289
From: libsdl-org/SDL#5901