Skip to content

build: add soname to shared libraries #2521

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

build: add soname to shared libraries #2521

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ option (Seastar_DEPRECATED_OSTREAM_FORMATTERS
"Enable operator<< for formatting standard library containers, which will be deprecated in future"
ON)

set (Seastar_RELEASE_DATE
20221127
CACHE
STRING
"Last release date")

set_property (CACHE Seastar_RELEASE_DATE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need a property for the release date?

PROPERTY
STRINGS 20221127)

set (Seastar_API_LEVEL
"7"
CACHE
Expand Down Expand Up @@ -780,6 +790,15 @@ add_library (seastar
src/websocket/server.cc
)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use API level as soname to enable applications to select
and link agains shared libraries based on their SO versions.

Actually a neat idea. But note, API levels are only about source-level compatibility. It's possible for a function signature to change without change the API level.

Example: if we add a trailing default parameters to a function, it's still source compatible, but links against an older .so will fail.

Given this, is this still helpful for you?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am happy to follow up with another change. It would be nice not to carry patches, but yes ideally soname would reflect ABI compatibility, not API compatibility. Still learning about Seastar, so not sure how often ABI is broken in a backwards incompatible manner. If the release year and month is used, then this would always increment the soname, and it would be assumed that typically ABI is broken on each update.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One could run a tool such as libabigail when doing a release and then automatically bump the soname if there is an ABI change.

# Shared library soname
set_target_properties(seastar PROPERTIES
VERSION ${PROJECT_VERSION}.${Seastar_API_LEVEL}.${Seastar_RELEASE_DATE}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In semantic versioning (semver), a version number consists of three components: MAJOR.MINOR.PATCH. The major version number changes least frequently and indicates incompatible API changes. The minor version is incremented more often for backward-compatible feature additions, while the patch version is used for backward-compatible bug fixes.

The Seastar_API_LEVEL changes less frequently than the ABI version, as it is only incremented for significant API changes rather than ABI-breaking modifications.

PROJECT_VERSION is also composed of multiple component. see https://cmake.org/cmake/help/latest/variable/PROJECT_VERSION.html.

Based on the discussion above, I suggest:

  • PROJECT_VERSION should be treated as a versioning string with multiple components (major.minor.patch) rather than a single number.
  • Although Seastar_API_LEVEL is crucial for API versioning, it may not belong in the libraries' version numbers since we make ABI changes within the same Seastar_API_LEVEL.
  • Including the release date in the version number makes sense given our flexible release schedule.

SOVERSION ${PROJECT_VERSION})

# We disable _FORTIFY_SOURCE because it generates false positives with longjmp() (src/core/thread.cc)
set_source_files_properties(src/core/thread.cc
PROPERTIES COMPILE_FLAGS -U_FORTIFY_SOURCE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are partially reverting c96444f. probably want to drop this change?


add_library (Seastar::seastar ALIAS seastar)

add_dependencies (seastar
Expand Down Expand Up @@ -1170,6 +1189,10 @@ if (Seastar_INSTALL OR Seastar_TESTING)
src/testing/seastar_test.cc
src/testing/test_runner.cc)

set_target_properties(seastar_testing PROPERTIES
VERSION ${PROJECT_VERSION}.${Seastar_API_LEVEL}.${Seastar_RELEASE_DATE}
SOVERSION ${PROJECT_VERSION})

add_library (Seastar::seastar_testing ALIAS seastar_testing)

target_compile_definitions (seastar_testing
Expand All @@ -1189,6 +1212,9 @@ if (Seastar_INSTALL OR Seastar_TESTING)
include/seastar/testing/perf_tests.hh
tests/perf/perf_tests.cc
tests/perf/linux_perf_event.cc)
set_target_properties(seastar_perf_testing PROPERTIES
VERSION ${PROJECT_VERSION}.${Seastar_API_LEVEL}.${Seaster_RELEASE_DATE}
SOVERSION ${PROJECT_VERSION})
add_library (Seastar::seastar_perf_testing ALIAS seastar_perf_testing)
target_compile_definitions (seastar_perf_testing
PRIVATE ${Seastar_PRIVATE_COMPILE_DEFINITIONS})
Expand Down