On 23-07-2022 13:09, soe...@soeren-tempel.net wrote:
From: Sören Tempel<soe...@soeren-tempel.net>This patch is a fix for bug #49232 [1]. To summarize this bug, the current load-foreign-library implementation does not load versioned sonames (e.g. libfoo.so.5) which are common on Linux. This is an issue for us at Alpine Linux since we ship unversioned sonames (e.g. libfoo.so) separately. Please refer to the original bug report for details. [...]
Long term, I think it would be ideal for Guile to decide upon a major version (and maybe even location, depending on the choices of the distro) at _compile_ time instead of runtime, not unlike how other compilers work.
More concretely, we could have a macro 'link-foreign-library', of the form: (link-foreign-library library [maybe other arguments).If the Guile module is being compiled with --rpath, it searches $CROSS_LIBRARY_PATH or $LIBRARY_PATH and encodes the full file name (/usr/lib/.../libguile-... or /gnu/store/.../lib/...) in the .go, which avoids some manual patching we have to do in Guix.
It could also detect if it's loading a libfoo.so pointing to libfoo.so.N, and if so, replace libfoo.so by libfoo.so.N.
The former (rpath) would work for Guix, and I expect the latter (libfoo.so -> libfoo.so.N) to work on Alpine.
IIUC, the string-prefix? search is non-deterministic, which can make debugging complicated when multiple versions are installed. I think it would be better to bail out if there are multiple matches instead of a risk of guessing incorrectly.+(define (file-exists-in-dir-with-extension dir basename exts) + (let* ((dir-stream (opendir dir)) + (ret (let loop ((fn (readdir dir-stream))) + (and (not (eof-object? fn)) + (if (filename-matches-with-extension? fn basename exts) + (in-vicinity dir fn) + (loop (readdir dir-stream)))))))
The prefixing behaviour is also not documented, so some documentation is needed in the manual.
Questions: * Does it go scanning the dir even if libfoo.so could be found? Otherwise, there are some possible performance gains by checking for libfoo.so first -- consider the case where /usr/lib is huge. * When doing (load-foreign-library "/gnu/store/.../libfoo.so") (absolute file name!), would it search for /gnu/store/.../libfoo.so.N? If so, that would be surprising, especially if libfoo.so.N exists. * If doing libfoo.so.N, will it search for libfoo.so.N.M? * Does it only apply to the system paths, or also to GUILE_SYSTEM_EXTENSION_PATH, LTDL_LIBRARY_PATH and GUILE_EXTENSION_PATH? The latter would be surprising to me, as versioning is more of a system thing. * To test it, and avoid breaking things later with future changes to load-foreign-library, could some tests be added? * Is this change desirable? I mean, this is an FFI API, so the ABI of the library is rather important. If a Guile module links to libfoo.so, and they had version N in mind, then it's important it doesn't link to N-1 or N+1 instead, because of ABI incompatibilities. As such, to me it seems _good_ that you got some errors, as now you get a reminder to explicitly state which ABI version is needed. (YMMV, and the mileage of the Guile maintainers might vary, etc.)Also, this seems like a non-trivial change to me, so a copyright line might be in order, unless you did the copyright assignment.
Greetings, Maxime.
OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
