-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Support versioned dylibs #90260
Support versioned dylibs #90260
Conversation
Right now, rustc fails when trying to link against a versioned dylib crate (e.g. libsomecrate.so.0.1). This patch tries to address this by replacing the simple ends_with dll_suffix check with a better one.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @wesleywiser (or someone else) soon. Please see the contribution instructions for more information. |
rust/compiler/rustc_codegen_ssa/src/back/link.rs Line 1288 in 84c2a85
|
This comment has been minimized.
This comment has been minimized.
60b7dfa
to
ca1a70b
Compare
ca1a70b
to
aa22549
Compare
This comment has been minimized.
This comment has been minimized.
Due to the fact that if we were not using verbatim linking, linking against a versioned crate will only work if a unversioned symlink to the versioned one is part of the searchpath. This is "preliminary" as right now it only uses the whole filename for GNU-style linkers. It should however, be expanded to any linker that supports verbatim linking. Also added suggestions from @wesleywiser.
aa22549
to
15d33db
Compare
Why does meson include a suffix here? That seems like it gives the impression that there's meaningful library versioning and compatibility. |
it does so for any shared_library() regardless of language |
Can you set version to an empty string to disable this? Or will meson give something like |
no, its quite strict about that.
|
That doesn't seem like an issue we should be working around, here. dylibs don't have version numbers; they're just a As an example, Debian ships a dynamic If we were to change this to work around the issue, I don't think we should just change the expected suffix to omit this error. At most, I think we should offer a |
well, exactly because file-extensions inherently don't mean anything on POSIX, someone (I assume the GNU Developers) figured they can use the suffix for versioning. Thus relying on the suffix is inherently flawed in POSIX environments. Letting the user specify the format works, but to me it seems somewhat unnecessary, because having the version suffix behind the extension is just the common way to do dynamic versioning since decades. |
The crate version is inherently meaningless wrt to the abi in rust. Any change to the imlementation of a crate will change it's SVH (semantic version hash) and potentially the rest of the crate metadata. This is not only when changing the actual source files (which a human readable version could account for) but also the set of
This is not true. Many programs rely on the file extension. Take for example your editor determining which language to use for syntax highlighting, your file explorer chosing which application to open your files with or even gcc determining which compiler it needs to use. (the C compiler, C++, Pascal, Fortran, ...) It is true that sometimes the extension doesn't matter and instead magic bytes are used, but this is not always possible (think text file) or unambiguous (think .docx or .odt files, both of which are zip files) Rustc needs the file extension to distinguish between rust metadata files (.rmeta), rust libraries (.rlib), non-rust static libraries (.a) and dynamic libraries (.so/.dylib) (honestly it may have been a good idea to use a different extension for rust dylibs to alow both the dylib and cdylib crate types at the same time and to avoid accidentally trying to use a non-rust dylib as a rust dylib) |
it might have, because I've noticed there's quite often confusion between dylib and cdylib crates, however I fear that on systems where the dynamic object loader cares about file-extensions (win nt) it'd cause a lot of trouble. |
I guess so. Nothing we can do about it now anyway. |
rustc has two ways to lookup crates:
The first way has the same restrictions on file extensions as the second way, but they seem artificial. I feel somewhat skeptical about recognizing the |
Allowing them for the first way would result in confusing errors when using a crate that has such a dependency as dependency. Indirect dependencies use the second way for lookup. |
well, almost all libraries are stored like this in linux (a libname.so.1.2.3, with symlinks to it: libname.so.1 and libname.so, with the second one usually provided by its devel package), so yeah the dynamic linker uses it. this obv. throws up the question how precompiled rust dependencies could be stored system wide |
For libstd and libtest this is In rust a human readable version string doesn't make sense the way it does in C. In C it is possible to precisely determine which changes break ABI compatibility and changing the compiler version doesn't usually change the ABI. For rustc changing the compiler version or even a single byte in the crate source will break ABI and rustc will give you an error when trying to compile. Only changing the compiler version is currently guaranteed to give an error at runtime, but any source changes may do this too or cause corruption. |
Ping from triage: When it's ready for review send a message containing |
I was wanting to implement what @joshtriplett suggested:
maybe along the lines of allowing something like
again, because when I tested it I must have done something wrong, because it didn't for me or otherwise no error wouldn't even have occurred. but so far I just haven't found the time to do so :/ |
Thank you for the response, @eli-schwartz; much appreciated. It sounds like it is possible to build a shared library with meson that doesn't have a version number appended, and to reference such a shared library, which would be the correct answer for a Rust dylib library (since there's currently no ABI that would allow for a meaningfully versioned shared library). Given that, it doesn't sound like we'd need to support the versioned case, unless I've missed something here. @sp1ritCS I would suggest first confirming with current meson that it's possible to build and consume an unversioned shared library. Assuming it is, I think this can be closed, unless there's a specific use case motivating the force option. |
So I've re-tested rustc 1.58.0 with meson 0.60.3.
it is, when soversion is not set. However I want to have versioned dylibs to allow for having multiple versions of the same dependency installed, to accommodate breaking changes in them for different dependents that have fixed a specific version. to allow that I see three different possibilities:
|
The way cargo handles this is by naming them like |
Meson very explicitly creates such a symlink, see https://github.com/mesonbuild/meson/blob/7528c69fcca63889d67c1d6c2463c9e9485585a8/mesonbuild/backend/ninjabackend.py#L3088-L3101
This is completely wrong and invalid. You are making statements that are highly dependent on which particular Linux distro you use, and doesn't apply to people like me, who do not use Debian. My distro does not have devel packages, the versionless symlink is traditionally provided by the same package that provides the main library file. Even then, meson provides and installs the symlink. Debian uses the symlink that meson installs. Just like they use the symlinks that CMake or autotools would install. It's just that Debian happens to put some of the installed files in one dpkg archive, and some of them in a different dpkg archive. But that is a pointless implementation detail that has no earthly relevance here. If you are compiling against a library I presume you would of course install the devel package on Debian.
Implying that CMake and autotools are also breaking multi-version installs? Why has that never been a problem anywhere other in the 21st century computing experience other than this one ticket in this one (rust) project for this one (you) participant in the ticket? (Multi-version installs just have the unversioned symlink point at the version of the library which the headers correspond to. You can also only have a single non-versioned set of headers in /usr/include, after all.) Meson's behavior of creating symlinks in the install tree is absolutely required, because if Meson did not do this (and CMake did, and autotools did, and it is a mandatory requirement of every single project using shared libraries with versions, and also a mandatory requirement of every single project such as a distro that builds and packages "software")... ... then Meson would be a joke of a build system, and no one would use something that doesn't even get the basic fundamentals of a build system correct. :) So, they are definitely there at install time (except on systems where symlink creation does not work, in which case Meson logs an informative error to the console, and autotools maybe just logs They are also there at build time, which isn't strictly required in order to get the basic fundamentals of a build system correct, but, Meson does it anyway for robustness purposes, e.g. to support building against uninstalled projects (that is also why we create So, I really do not understand under what conditions you are getting "the alias symlink is not created". Do you have a sample project that you've verified the issue with? What operating system are you using? How do I replicate your issue? I confess I'm utterly unable to get Meson to not create a symlink. |
Well, it has. Assuming the dpkg archives are named correctly, it is usually possible to install multiple liblibrary-X packages together, but just having the latest library-devel installed. On some Distros (I'd assume ones that use pacman), like the one you use yourself, this is just not possible as there are conflicting files like the unversioned symlink and the pkg-config and headerfiles which are provided by the separate library-devel package on most dpkg and rpm based distros.
I was thinking that compiling against a outdated library would only occur in separate chroots/VMs/containers that have the minimal dependency footprint to build whatever package is required, so installing an outdated library-devel package there would not be an issue. But now do realize that that is flawed, as there (a) are users that want to build dependents on their system directly and (b) there are cases where two dependencies depend on the same crate but with different versions in rust. I wouldn't know how to solve that.
yes, just running make install will not allow multi versioning. I've never said that it wouldn't. multi-versioning will have to be provided by dpkg/rpm by splitting subpackages that do not share common files between versions.
I'll attach my test tree I created today. It's basically just a modified version of the meson rust executable template and a subproject with the meson rust library template, but as shared- instead of static_library and with a soversion set. Commenting out
and the symlink is nowhere to be found in the builddir (the log also doesn't contain anything suspicious):
|
Thank you for the test case! I've reproduced the issue, it is absolutely a bug in meson, and I have written a fix. (It is specific to the versioned dylib itself being built with specific languages, in this case rust -- C/C++ dylibs generate symlinks just fine.) |
Basically the last thing we did during target processing was to generate shlib symlinks for e.g. libfoo.so -> libfoo.so.1. In some cases we would dispatch to another function and return early, though, which meant we never got far enough to generate the symlinks. This then led to breakage when people tried to compile against libfoo.so This surely breaks -uninstalled.pc usage, and also caused problems in rust-lang/rust#90260
Note that even with the restoration of the symlink, I guess either |
Basically the last thing we did during target processing was to generate shlib symlinks for e.g. libfoo.so -> libfoo.so.1. In some cases we would dispatch to another function and return early, though, which meant we never got far enough to generate the symlinks. This then led to breakage when people tried to compile against libfoo.so This surely breaks -uninstalled.pc usage, and also caused problems in rust-lang/rust#90260
So I've tried implementing a way to force rustc to treat a specific --extern as one kind of dependency. See: sp1ritCS@49e1fd8 . One can get meson to use this with the following simple change: diff --git a/mesonbuild/backend/ninjabackend.py b/mesonbuild/backend/ninjabackend.py
index 75dd535e8..2b7bfeab4 100644
--- a/mesonbuild/backend/ninjabackend.py
+++ b/mesonbuild/backend/ninjabackend.py
@@ -1716,10 +1716,10 @@ class NinjaBackend(backends.Backend):
for d in target.link_targets:
linkdirs.add(d.subdir)
if d.uses_rust():
- # specify `extern CRATE_NAME=OUTPUT_FILE` for each Rust
+ # specify `extern kind:CRATE_NAME=OUTPUT_FILE` for each Rust
# dependency, so that collisions with libraries in rustc's
# sysroot don't cause ambiguity
- args += ['--extern', '{}={}'.format(d.name, os.path.join(d.subdir, d.filename))]
+ args += ['--extern', '{}:{}={}'.format(d.rust_crate_type, d.name, os.path.join(d.subdir, d.filename))]
elif d.typename == 'static library':
# Rustc doesn't follow Meson's convention that static libraries
# are called .a, and import libraries are .lib, so we have to
with that, the demo I've sent builds perfectly (and runs after having added the libstd to the ld searchpath :)) |
That sounds great, either your patch or one to use the unversioned symlink will be more than welcome as a Meson PR. |
☔ The latest upstream changes (presumably #93655) made this pull request unmergeable. Please resolve the merge conflicts. |
Basically the last thing we did during target processing was to generate shlib symlinks for e.g. libfoo.so -> libfoo.so.1. In some cases we would dispatch to another function and return early, though, which meant we never got far enough to generate the symlinks. This then led to breakage when people tried to compile against libfoo.so This surely breaks -uninstalled.pc usage, and also caused problems in rust-lang/rust#90260
@sp1ritCS any updates on this? |
@Dylan-DPC I expect a comment regarding
if the Rust developers consider this adequate. |
@Dylan-DPC has there been any change since I wrote that last comment, or how is the status of this? |
I would still prefer for rustc to not be aware of the |
that is what I implemented in that linked commit. it allows crates to be tagged with |
Do you mean sp1ritCS@49e1fd8? A higher level build system would be a better place for addressing this than rustc, it can use whatever naming it wants and then satisfy rustc with symlinks. The PR is sitting for too long with a |
that is part of the problem. rustc is already quite high-level, given that it invokes the linker itself. If rustc could just output object files and let them be linked by a higher level build system, like a normal compiler, this wouldn't be an issue in the first place.
to my understanding, it does actually work with indirect dependencies, if they are also specified with --extern, liked pkg-config would do. |
|
Right now, rustc fails when trying to link against a versioned dylib crate (e.g.
libsomecrate.so.0.1
). This can happen if you use mesonbuild/meson as build system rather than cargo and you add alibrary
(a dylib for cargo) as dependency.This patch tries to address this by replacing the simple
ends_with
dll_suffix
check with a better one.