-
-
Notifications
You must be signed in to change notification settings - Fork 59
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
More granular SONAME versioning #345
Comments
You are right, we'll have a new cargo soon and I'd release your improvement/fix with it if is fine for you. If you prefer having it in a point release please tell me and I'll prepare it :) |
Sounds great, thank you! :) Looking forward to the next cargo release. |
The new cargo is out, let me know if everything works as you need. |
This appears to break the default case where the soname version isn't specified in metadata. For example, a library I'm building with cargo-c now has |
in your case the soversion should be |
That makes sense, I didn't revert to the previous version to verify my guess. |
I'll try to refactor and cleanup a bit more since there is one more corner case:
This is what I'd expect, would it work for you? |
I believe that should work, thanks for looking into it. |
Since no package in Arch Linux is using rav1e now ships as I noticed the patch in #350 is still essential because otherwise it's still creating a Besides that everything seems to be fine, thanks for working on this! |
This causes a regression in pkgsrc, at least on illumos platforms. It broke somewhere around 0.9.26, and I've just verified that it is still broken in 0.9.27. For example, building libimagequant, I end up with the following library and symlinks: $ ls -l libimagequant.so*
lrwxrwxrwx 1 root root 22 Oct 11 20:18 libimagequant.so -> libimagequant.so.0.0.0
lrwxrwxrwx 1 root root 22 Oct 11 20:18 libimagequant.so.0 -> libimagequant.so.0.0.0
-rwxr-xr-x 1 root root 3008816 Oct 11 20:18 libimagequant.so.0.0.0 However, the $ elfdump libimagequant.so.0.0.0 | grep SONAME
[9] SONAME 0x76a1e libimagequant.so.0.0 This causes problems with linking against this library, as can be seen during the build of This was working fine with older releases, so if you could resolve the regression that'd be great. I'm happy to test any proposed changes. Thanks! |
Hmm, there's a chance that |
Ok, so after ensuring it was rebuilt with 0.9.27 it does appear as though the $ ls -l libimagequant.so*
lrwxrwxrwx 1 root root 22 Oct 12 12:29 libimagequant.so -> libimagequant.so.0.0.0
-rwxr-xr-x 1 root root 3008816 Oct 12 12:29 libimagequant.so.0.0.0
$ elfdump libimagequant.so.0.0.0 | grep SONAME
[9] SONAME 0x76a1e libimagequant.so.0.0.0 though there is now no longer a Thanks though for fixing the original regression, and apologies for the incorrect original comment. |
@jperkin I've investigated this and it's unrelated to cargo-c, I've submitted a patch to libimagequant. |
Version in libimagequant is intentional. I do not want to create |
Could you make it |
I've set it to |
I'm setting the soname to fit the semver so:
|
Imposing a notion of semver into soname versions is a bit strange and unexpected, as this completely violates how shared C library versioning normally works. I feel like this needs to be very clearly and obviously documented. @kornelski The above means that making a release with the version set to |
(And for the record, it also means that it's explicitly not compatible with the earlier |
That's also the (cargo) semver meaning of the versions though. 0.0.5 is incompatible with 0.0.4. |
Semver is fairly clear on what to expect, so if you do otherwise you aren't following semver.
I asked for a version |
Sure, but it's strange and unusual to impose this onto the concept of ABI soname versions, which are intentionally wholly separate from crate versions, without documenting this.
|
Assuming this is about https://bugs.archlinux.org/task/80209, this package was initially missed in https://archlinux.org/todo/cargo-c-sover-rebuild/ because it's >= 1.0.0 and assumed to be unaffected by this change (just like libdovi). By the time I noticed The concept of There are no concerns about |
It's indeed a well-understood concept among Rust devs for Rust crates. But we're not talking about Rust crate ABIs here, we're talking about C-ABI shared libraries, and if the intention is to build libraries to be used from languages other than Rust then this line of reasoning doesn't make any sense because it's just not how C-ABI shared libraries normally work.
Not if we're talking about C-ABI shared libraries, which appears to be the claimed purpose of |
When you break the API you always break also the ABI, I'm not sure what's the problem that you are arguing about so vehemently. |
I'm saying that for C-ABI shared libraries the switch from Hard-coding the current policy that it is a breaking change is imposing a notion that does not exist in the C-ABI shared library ecosystem, which makes it difficult to use |
For any software that follows semver, it means exactly that. (but I would be quite displeased as consumer of such theoretical library) |
For C shared libraries, versions have no meaning at all in general and every library does its own thing. But this here is about mapping a cargo crate version to a soname, and cargo crate versions have a very specific meaning. |
@kyrias I think you're mixing up the C ABI (a calling convention for assembly) with dynamic linking (load a shared object by name). Both are not unique to C and can be implemented/integrated in most languages (like cargo-c did). Neither system has a concept of "you MUST use .so.0, .so.1, .so.2", this is simply what most C projects ended up doing. This SONAME is controlled by upstream (both in C and Rust world) and cargo-c provides two interfaces to configure this, either by auto-detecting it from the crate version What is a valid concern though, I think, if I have a C project that currently uses [package.metadata.capi.library]
version = "17.0.0" To get When planning this feature we should also take into account there's also the librustls use-case of "I don't want to commit to any ABI and every new version should be considered ABI breaking (for safety reasons)". To support both, there could be a setting like:
This setting should be in I think this is more of an oversight instead of "cargo-c not caring about ABI compatibility", and also doesn't have much to do with the Arch Linux bugreport, except it would've allowed the libimagequant project to specify in their Cargo.toml that they want to keep using |
Related to rustls/rustls-ffi#345 and rustls/rustls-ffi#274.
I've tried to use cargo-c together with Arch Linux packaging tools, which is able to automatically detect and embed the SONAME into package metadata, but noticed I get this:
After looking into how this works I noticed it's using a string embedded in the .so file:
I found the relevant part in the cargo-c codebase:
cargo-c/src/target.rs
Lines 81 to 85 in 0819def
There are two things about this:
0.11.0
, I think the soname should belibrustls.so.0.11
>= 1.0.0
, I think it should be possible to configure a major.minor (or even major.minor.patch) SOVERSpecifically with librustls I would like to be able to build it with
-Wl,-soname,librustls.so.0.11.0
until the developers are comfortable making ABI commitments. :)The text was updated successfully, but these errors were encountered: