From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?S=C3=B6ren?= Tempel Newsgroups: gmane.lisp.guile.devel,gmane.lisp.guile.bugs Subject: Re: [PATCH] load-foreign-library: perform substring match on library files Date: Sun, 24 Jul 2022 14:16:25 +0200 Message-ID: <2EA1MELO27342.28JGAL8QO05T4@8pit.net> References: <23TGNISPS9PPL.2YOENNQD9V56X@8pit.net> <20220723110919.30107-1-soeren@soeren-tempel.net> <936e98a7-a9d2-2607-ef6a-8c0edff19c4a@telenet.be> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="34827"; mail-complaints-to="usenet@ciao.gmane.io" Cc: bug-guile@gnu.org, guile-devel@gnu.org To: Maxime Devos Original-X-From: guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org Sun Jul 24 14:17:10 2022 Return-path: Envelope-to: guile-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oFaXl-0008oK-U1 for guile-devel@m.gmane-mx.org; Sun, 24 Jul 2022 14:17:10 +0200 Original-Received: from localhost ([::1]:49964 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oFaXh-00024V-Q5 for guile-devel@m.gmane-mx.org; Sun, 24 Jul 2022 08:17:05 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:55652) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oFaXI-00022b-Ig; Sun, 24 Jul 2022 08:16:40 -0400 Original-Received: from magnesium.8pit.net ([45.76.88.171]:8213) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oFaXF-0007Fs-Ro; Sun, 24 Jul 2022 08:16:40 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; s=opensmtpd; bh=h7qK54DA36 y71eN1MQQuKsXvDhM1TBozg+jaCvi95zY=; h=in-reply-to:references:from: subject:cc:to:date; d=soeren-tempel.net; b=JNKU7oNr14IHj5a98X7/vG2pnuU B1AdO8uBn9vqGtSxenzHwCMXkuIRB6XpFOce1pRVU7mtntrEagXfyJQ+LBbMMyqA0B6AOe 7NGROFfxjj/H6j47mhpO8ywzulG0UQ2mvy20YudrIWHfk5saYFo59+wMsxprt3m++ar6XK jNZw= Original-Received: from localhost (p200300f5ff05aa00efb8aa09824b98d4.dip0.t-ipconnect.de [2003:f5:ff05:aa00:efb8:aa09:824b:98d4]) by magnesium.8pit.net (OpenSMTPD) with ESMTPSA id 605b84b6 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:YES); Sun, 24 Jul 2022 14:16:30 +0200 (CEST) In-Reply-To: <936e98a7-a9d2-2607-ef6a-8c0edff19c4a@telenet.be> Received-SPF: pass client-ip=45.76.88.171; envelope-from=soeren@soeren-tempel.net; helo=magnesium.8pit.net X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org Original-Sender: "guile-devel" Xref: news.gmane.io gmane.lisp.guile.devel:21271 gmane.lisp.guile.bugs:10308 Archived-At: Hi, Thanks for your feedback, comments below. Maxime Devos wrote: > Long term, I think it would be ideal for Guile to decide upon a major=20 > version (and maybe even location, depending on the choices of the=20 > distro) at _compile_ time instead of runtime, not unlike how other=20 > compilers work. Sure, that could work too. > 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. What kind of manual patching do you do on Guix? Could you refer me to the code for that? Maybe that's something we could also do on Alpine in the meantime. > IIUC, the string-prefix? search is non-deterministic, which can make=20 > debugging complicated when multiple versions are installed.=20 Well, on Linux readdir(3) file name order depends on the file system implementation. Since the algorithm returns the first file with a substring match, it depends on the readdir order. As long as the same filesystem is used, it should be deterministic. > I think it would be better to bail out if there are multiple matches > instead of a risk of guessing incorrectly. Many packages provide multiple .so files with different version granularity so bailing out if there are multiple substring matches doesn't really work. For example, for libgit2 I have the following files installed on my system: $ ls /usr/lib/libgit2* /usr/lib/libgit2.so.1.4 /usr/lib/libgit2.so.1.4.4 Where the former is a symlink to the latter. However, it would be possible to collect all substring matches and prioritize them according to some algorithm (e.g. alphabetical order). This would also make the algorithm independent of the readdir(3) order. > The prefixing behaviour is also not documented, so some documentation is=20= > needed in the manual. I can add some documentation if there is interest in merging this. > * 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. Yes, a fast path could be added though you probably really need to have a lot of files installed in /usr/lib for this to be worth it. > * When doing (load-foreign-library "/gnu/store/.../libfoo.so") > (absolute file name!), would it search for > /gnu/store/.../libfoo.so.N?=C2=A0 If so, that would be surprising, > especially if libfoo.so.N exists. Yep, it does. I originally didn't want to modify the handling of absolute paths but unfortunately during testing I noticed that Guile extensions seem to be loaded with an absolute path and hence don't work without the libfoo.so symlink [1]. > * If doing libfoo.so.N, will it search for libfoo.so.N.M? Yes, since libfoo.so.N is a prefix of 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. If those paths are also searched using the load-foreign-library procedure then they are affected by this change. Also, I am not a Guile expert but on Alpine, Guile extensions such as guile-reader also ship versioned sonames [1]. > * To test it, and avoid breaking things later with future changes to > load-foreign-library, could some tests be added? Probably, though I am not familiar with the Guile test setup and there don't seem to be any existing tests for foreign-library. > * Is this change desirable?=C2=A0 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.) In my experience, most languages which don't link against shared libraries directly but instead load them at run-time don't hardcode ABI versions (for example, refer to Python's ctypes.util.find_library). Also, the current implementation of load-foreign-library does not force you to specify an ABI version but instead loads whatever the libfoo.so symlink refers to. > Also, this seems like a non-trivial change to me, so a copyright line=20 > might be in order, unless you did the copyright assignment. I didn't do any copyright assignment yet but if there is actually any interest in merging this then I can do it once we agreed on changes to the algorithm. Greetings, S=C3=B6ren [1]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/12783