-
Notifications
You must be signed in to change notification settings - Fork 97
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
Homebrew dylib version is wrong #52
Comments
Fixed in 076b491 |
Shouldn't this have been something like: -set_target_properties(cbor_shared PROPERTIES OUTPUT_NAME cbor VERSION "0.0.0" SOVERSION 0)
+set_target_properties(cbor_shared PROPERTIES OUTPUT_NAME cbor VERSION ${CBOR_VERSION} SOVERSION 0) Or perhaps (just picking a number, 6 looked related) -set_target_properties(cbor_shared PROPERTIES OUTPUT_NAME cbor VERSION "0.0.0" SOVERSION 0)
+set_target_properties(cbor_shared PROPERTIES OUTPUT_NAME cbor VERSION ${CBOR_VERSION} SOVERSION 6) Or did you really mean to make the soname the full
|
Sorry, looks like I guessed wrong how Usually the SONAME would be something like Since you have the SONAME set to |
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -14,7 +14,7 @@ endif()
add_library(cbor STATIC ${SOURCES})
add_library(cbor_shared SHARED ${SOURCES})
-set_target_properties(cbor_shared PROPERTIES OUTPUT_NAME cbor VERSION ${CBOR_VERSION} SOVERSION ${CBOR_VERSION})
+set_target_properties(cbor_shared PROPERTIES OUTPUT_NAME cbor VERSION ${CBOR_VERSION} SOVERSION 6)
configure_file(libcbor.pc.in libcbor.pc @ONLY)
The above produces:
And we have:
That's more inline with what I would expect sonames to be like. |
Thanks for bringing this up @panlinux, I didn't think that through. My first hunch is that if we live in a semver world, the current behavior is correct because there are no compatibility guarantees for the 0.x version. In practice, minor versions have been breaking and patches should still be backwards compatible to keep things sane (filed #130 to track). My proposal is to set the the SOVERSION to ${CBOR_VERSION_MAJOR}.${CBOR_VERSION_MINOR} until for we reach 1.0.0 (which should happen soon given the age of the project and the growing amount of dependent code) to make sure breaking changes are correctly tracked by minor versions. This is far from perfect, minor releases will still break clients, but it does ensure correctness and eases the pain at least a bit. When we reach 1.0.0, SOVERSION should be just CBOR_VERSION_MAJOR and the world will be all sunshine again. An alternative would be to manually track the ABI compatibility in a separate version label, which would give us idiomatic shared lib version numbers immediately, but I would like to avoid the SOVERSION drifting away from CBOR_VERSION_MAJOR to prevent confusion. WDYT, is that reasonable? Or am I thinking about it in a wrong way? |
Ok, that patch gives us this:
And:
Patch releases (0.6.1, 0.6.2, etc) would keep the above, and not break clients? I'm assuming: |
Yes, seems like the cleanest way out of this. |
That sounds good. I applied this patch to the ubuntu libcbor package and it's going through review. We would however be releasing a libcbor 0.6.0 package with this patch, which makes it quite different from what you released as 0.6.0. I'm not sure if this warrants a new upstream release, or if you are ok with us having a 0.6.0 package that differs in its soname from your upstream tarball. |
My guess would be that people who build from source would link statically, so I don't see it as a major issue. FWIW, I can also release a "0.6.1" patch to conceal the tracks 😄 You folks could pull it at your convenience and it will be compatible |
After a fresh install:
The text was updated successfully, but these errors were encountered: