-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
build: enable Seastar to build shared and static libs in a single build #2427
base: master
Are you sure you want to change the base?
Conversation
cea6dbc
to
0508fb3
Compare
@denesb could you please help review this change? it implements the proposal at scylladb/scylladb#2717 (comment) |
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.
Looks good, I don't understand the Cmake changes in detail, but the concept as explained in the cover letter as well as the code in high level looks good.
thank your Botond, for your generous review. |
@scylladb/seastar-maint hello maintainers, could you help review this change? |
before this change, we respect the CMake variable named `BUILD_SHARED_LIBS`, and build shared libraries if it is set, build static libraries otherwise. but this model cannot fulfill the needs of a parent project which needs to build both static and shared seastar libraries with different configuration in a multi-config generator settings. as `BUILD_SHARED_LIBS` is a CMake variable which cannot be changed at build time, and hence cannot be assigned with different values using generator expression. so, in this change, we add two options - Seastar_BUILD_STATIC_LIBS - Seastar_BUILD_SHARED_LIBS to configure if the build should generated static libraries and shared libraries respectively. but `BUILD_SHARED_LIBS` is still supported, and the behavior is backward compatible. the only user-visible differences are, the libraries of - seastar - seastar_testing - seastar_perf_testing are now aliases of the targets which builds the static or shared libraries. so one cannot build "seastar" as a target anymore, but should specify which library to build: ``` $ cmake --build build --target seastar # does not work anymore $ cmake --build build --target seastar-static $ cmake --build build --target seastar-shared ``` the reason is the limit of an ALIAS library, which cannot be used as a target. but we still need to use a non-interface library to generate .pc file, where, for instance, we use `$<TARGET_FILE_NAME:seastar_testing>` for the library list of a certain Seastar library. but a CMake-based project including seastar can still link against it using "seastar" as a library. Refs scylladb/scylladb#2717 Signed-off-by: Kefu Chai <[email protected]>
0508fb3
to
da15e0c
Compare
v2:
|
@scylladb/seastar-maint hello maintainers, could you help review this change? |
before this change, we respect the CMake variable named
BUILD_SHARED_LIBS
, and build shared libraries if it is set, build static libraries otherwise. but this model cannot fulfill the needs of a parent project which needs to build both static and shared seastar libraries with different configuration in a multi-config generator settings. asBUILD_SHARED_LIBS
is a CMake variable which cannot be changed at build time, and hence cannot be assigned with different values using generator expression.so, in this change, we add two options
to configure if the build should generated static libraries and shared libraries respectively. but
BUILD_SHARED_LIBS
is still supported, and the behavior is backward compatible. the only user-visible differences are, the libraries ofare now aliases of the targets which builds the static or shared libraries. so one cannot build "seastar" as a target anymore, but should specify which library to build:
the reason is the limit of an ALIAS library, which cannot be used as a target. but we still need to use a non-interface library to generate .pc file, where, for instance, we use
$<TARGET_FILE_NAME:seastar_testing>
for the library list of a certain Seastar library.but a CMake-based project including seastar can still link against it using "seastar" as a library.
Refs scylladb/scylladb#2717
Signed-off-by: Kefu Chai [email protected]