-
Notifications
You must be signed in to change notification settings - Fork 98
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
Add SONAME/SOVERSION to generated library #231
Conversation
Might be more robust to pull from https://github.com/evaleev/libint/blob/master/configure.ac#L3 akin to https://gitlab.com/libxc/libxc/-/blob/master/CMakeLists.txt#L125-135 and https://gitlab.com/libxc/libxc/-/blob/master/CMakeLists.txt#L473-474 . I don't think the |
Well I didn't realize the top-level Also note that libxc does not seem to use |
One more comment: As Also, I guess the future is all-CMake, so this is a stop-gap measure. I would agree that it is pretty brittle if we would have to use it for a long time and always remember to bump the library version in both places, but this sounds like a one-off (or two-off) think, famous last words... |
@mbanck thanks ... I tried to understand what
|
SOVERSION is part of the SONAME. It should be bumped at least every time the ABI changes in a not-backwards compatible way. In case symbols are not versioned (e.g. using a linker version script, as is the case here), the SOVERSION should also be bumped every time the ABI is changed in a backwards compatible way (e.g. when adding new functions). The library configuration is an orhogonal aspect. IMHO each configuration should have a different SONAME, and multiple libraries could be installed in parallel. According to #190, there are 20 different configurations, though only 5 seem to be actually used. E.g. psi4 could link to libint2_gss. |
@loriab bumping this to revisit in the light of planned use of
|
@@ -194,7 +194,7 @@ endif() | |||
# shared and static libraries built from the same object files | |||
if (LIBINT2_BUILD_SHARED_AND_STATIC_LIBS) | |||
add_library(libint2 SHARED $<TARGET_OBJECTS:libint2_obj>) | |||
set_target_properties(libint2 PROPERTIES LIBRARY_OUTPUT_NAME int2) | |||
set_target_properties(libint2 PROPERTIES LIBRARY_OUTPUT_NAME int2 VERSION 2.0.4 SOVERSION 2) |
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.
would ${LIBINT_VERSION}
would for VERSION
?
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.
As the API/ABI are not strictly controlled for backwards compatibility, the SOVERSION should be the same as the VERSION (better safe than sorry ...). This is the CMake default if SOVERSION is omitted.
set_target_properties(libint2 PROPERTIES
LIBRARY_OUTPUT_NAME int2
VERSION ${LIBINT_VERSION}
)
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.
Thanks. I think SOVERSION 2
is appropriate, the ABI of the library has been stable for many years.
good starter-level explanation of |
I don't think this PR thoroughly added SOVERSION (see below for mac/linux with BUILD_SHARED_LIBS=ON). I haven't fully investigated, but the PR may have just caught the build-both-static-and-shared target.
|
Fixes #229