Skip to content
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 library as part of another product without conflicts #386

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

AndreyAlifanov
Copy link
Contributor

@AndreyAlifanov AndreyAlifanov commented Sep 5, 2024

We need to build clickhouse-cpp as part of our product.
But CMakeLists.txt uses global options, sush as BUILD_SHARED_LIBS, BUILD_TESTS and so on.
This breaks our build system.

So I made changes in CMakeLists.txt.
If clickhouse-cpp is built as part of another product, global options are not changed by clickhouse-cpp.

Usage example. In this example flags touch only clickhouse-cpp, and don't affect our projects.

#outer CMakeLists.txt file
set (BUILD_SHARED_LIBS OFF)
set (BUILD_BENCHMARK OFF)
set (BUILD_TESTS OFF)
add_subdirectory(clickhouse-cpp)

@CLAassistant
Copy link

CLAassistant commented Sep 5, 2024

CLA assistant check
All committers have signed the CLA.

CMakeLists.txt Outdated
OPTION (BUILD_BENCHMARK "Build benchmark" OFF)
OPTION (BUILD_TESTS "Build tests" OFF)
OPTION (BUILD_SHARED_LIBS "Build shared libs" OFF)
IF (NOT CHCPP_EMBEDDED_BUILD)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe you need to have a corresponding option declaration clause (OPTION(CHCPP_EMBEDDED_BUILD ...)) before this if just to explain intention and simplify user's life.

Copy link
Collaborator

@Enmk Enmk Sep 6, 2024

Choose a reason for hiding this comment

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

Also, please consider changing option name to CLICKHOUSE_CPP_EMBEDDED_BUILD, like we have in 3.0: https://github.com/ClickHouse/clickhouse-cpp/blob/3.0/CMakeLists.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for answers and pieces of advice. I found another solution. It is not visible to user and handled automatically by build system.
I think, it will be better than new option.

@AndreyAlifanov AndreyAlifanov changed the title New CMake option - CHCPP_EMBEDDED_BUILD Build library as part of another product without conflicts Sep 9, 2024
@Enmk Enmk merged commit ca3ce41 into ClickHouse:master Sep 10, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants