-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
cmake: set the soname on the shared library #10023
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
Conversation
This will not be consistent with autotools on all platforms (I'd really like to know the reason libtool decides to have 100 different ways to compute this). But will work on normal Linux distributions, and it's quite open for extension. |
I think libtool works on more esoteric platforms than cmake does (or where our users will use it), so possibly this will not need the same amount of crazy logic. |
Sure. Nevertheless, I have identified following common platforms (each group use different calculations):
My old PR trying to solve this has been deemed too complicated. |
I suppose that answers the question why libtool has so many different ways...
This functionality is not possible to provide for all these platforms without a lot of complication. It is a complication I wish cmake itself provided for us, but since it doesn't I think we need to provide the logic if we ever want the cmake build to seriously become a serious autoconf alternative for curl builds.
I suppose this also means that my very simplified approach in this PR is then completely wrong for several of those groups? |
Tested this with curl-for-win and it did not break the trick used to suppress soname with autotools builds (libcurl dll versioning is not generally desired for standalone/native libcurl Windows builds). It also didn't disturb CMake Windows builds. So, it seems fine from these ends. |
Basically your changes will work correctly for the first group I mentioned. As for the others, the good question is if anybody really cares.
Well, CMake let's us set whatever SONAME/VERSION we want, it just does not deal with the autotools version scheme. I could try to submit my changes again, and see if you all would want it the codebase. Otherwise, it's probably better if it works at least partially, then not at all. But I'm wondering if an option to either build without SONAME, or to override it would be a good idea in this case. |
And therein lies the problem. The libtool version setup is made like to that to hide the platform complexities. It allows us to set one string and have that used automagically to set the correct number on numerous platforms. Without us having to know all those platforms. Without libtool since cmake doesn't help us here, we need to do that translation ourselves.
I think we should make an effort to do the right thing, yes! An option to switch it off or override is probably a good idea. |
I'll see if I can put something together this week. |
Unfortunately, life happens, and there's no way in hell I will be able to do anything till February. |
It might be a small step forward anyway. But how will this affect non-Linux builds? |
Not sure about Windows, but for other platforms the SONAME and/or file name may diverge from the one produced by the autotools. |
According to curl-for-win tests, the Windows CMake build is not affected. |
Does this mean that we cannot merge any fix for SONAME in cmake without trying to fix the entire world's set of platforms at once or should we merge the first take and let the ones you run into problems provide fixes? |
I think we shouldn't introduce an incorrect change. Maybe just guard your changes with a condition like: if(CMAKE_SYSTEM_NAME STREQUAL "AIX" OR
CMAKE_SYSTEM_NAME STREQUAL "Linux" OR
CMAKE_SYSTEM_NAME STREQUAL "GNU/kFreeBSD" OR
# FreeBSD comes with the a.out and elf flavours
# but a.out was supported up to version 3.x and
# elf from 3.x. I cannot imagine someone runnig
# CMake on those ancient systems
CMAKE_SYSTEM_NAME STREQUAL "FreeBSD" OR
CMAKE_SYSTEM_NAME STREQUAL "Haiku") ? |
Addresses KNOWN_BUG 15.1
... and make cmake use the components, and only set SONAME and VERSION for platforms we think this works on. Assisted-by: Jakub Zakrzewski
0b541d5
to
7643473
Compare
Thanks. Adapted to that, but also made the cmake version use the same numbers from the soname makefile with the necessary math. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the repeated properties, LGTM
- 898b012 curl#1288 - 5de6848 curl#10023 - bunch of others that are completed - `NTLM_WB_ENABLED` is implemented in a basic form, and now also scheduled for removal, so a TODO at this point isn't useful. And this 'to-check' item: Q: "The cmake build selected to run gcc with -fPIC on my box while the plain configure script did not." A: With CMake, since 2ebc74c curl#11546 and fc9bfb1 curl#11627, we explicitly enable PIC for libcurl shared lib. Or when building libcurl for shared and static lib in a single pass. We do this by default for Windows or when enabled by the user via `SHARE_LIB_OBJECT`. Otherwise we don't touch this setting. Meaning the default set by CMake (if any) or the toolchain is used. On Debian Bookworm, this means that PIC is disabled for static libs by default. Some platforms (like macOS), has PIC enabled by default. autotools supports the double-pass mode only, and in that case CMake seems to match PIC behaviour now (as tested on Linux with gcc.) Follow-up to 5d5dfdb curl#12500 Closes #xxxxx
- manual completed: 898b012 #1288 - soname completed: 5de6848 #10023 - bunch of others that are completed - `NTLM_WB_ENABLED` is implemented in a basic form, and now also scheduled for removal, so a TODO at this point isn't useful. And this 'to-check' item: Q: "The cmake build selected to run gcc with -fPIC on my box while the plain configure script did not." A: With CMake, since 2ebc74c #11546 and fc9bfb1 #11627, we explicitly enable PIC for libcurl shared lib. Or when building libcurl for shared and static lib in a single pass. We do this by default for Windows or when enabled by the user via `SHARE_LIB_OBJECT`. Otherwise we don't touch this setting. Meaning the default set by CMake (if any) or the toolchain is used. On Debian Bookworm, this means that PIC is disabled for static libs by default. Some platforms (like macOS), has PIC enabled by default. autotools supports the double-pass mode only, and in that case CMake seems to match PIC behaviour now (as tested on Linux with gcc.) Follow-up to 5d5dfdb #12500 Reviewed-by: Jay Satiro Closes #12509
Addresses KNOWN_BUG 15.1
Here we go again!