-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Feature/add cmake #420
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
Feature/add cmake #420
Changes from all commits
01f69db
d978c14
e21d574
2aec22e
ea913e1
09b2a2e
60c2c3d
abb6e6e
56b0b57
7aefe2c
618c061
4148af3
f814b25
9a2b130
0f700c7
c0ec895
82c6499
a2792d6
bd07a1f
e6f548b
66a8c73
a48d06d
7e63a31
e2c775d
2864b0f
b6b6dd3
5549fb3
35c4028
6fbf337
e48687a
379e707
d1bdccb
054bc95
78f2933
d861ada
6a8b2bc
3ad9225
466972d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| --- | ||
| # We'll use defaults from the Mozilla style, | ||
| # with 2 columns indentation. | ||
| BasedOnStyle: Mozilla | ||
| #TBD ContinuationIndentWidth: 4 | ||
| #TBD IndentPPDirectives: AfterHash | ||
| IndentCaseLabels: false | ||
| UseTab: Never | ||
| --- | ||
| Language: Cpp | ||
| # AccessModifierOffset: -4 | ||
| ### see BinPack... too | ||
| #XXX not yet possible with clang-format V7.0.1! CK | ||
| #XXX AllowAllArgumentsOnNextLine: true | ||
| #XXX AllowAllConstructorInitializersOnNextLine: true | ||
| #XXX AllowShortLambdasOnASingleLine: None | ||
| # | ||
| AllowShortBlocksOnASingleLine: false | ||
| AllowShortCaseLabelsOnASingleLine: false | ||
| AllowShortIfStatementsOnASingleLine: false | ||
| AllowShortLoopsOnASingleLine: false | ||
| #TBD AllowShortFunctionsOnASingleLine: None | ||
| # AllowShortFunctionsOnASingleLine: Inline | ||
| AllowShortFunctionsOnASingleLine: All | ||
| AlwaysBreakAfterDefinitionReturnType: None | ||
| AlwaysBreakAfterReturnType: None | ||
| # | ||
| ### Force pointers NOT to the type for C++. | ||
| # DerivePointerAlignment: false | ||
| # PointerAlignment: Right | ||
| #NO! SpaceBeforeCpp11BracedList: true | ||
| Cpp11BracedListStyle: true | ||
| FixNamespaceComments: true | ||
| # | ||
| BreakBeforeBraces: Custom | ||
| BraceWrapping: | ||
| AfterClass: true | ||
| AfterControlStatement: true | ||
| AfterEnum: false | ||
| AfterFunction: true | ||
| AfterNamespace: false | ||
| AfterObjCDeclaration: false | ||
| AfterStruct: true | ||
| AfterUnion: true | ||
| AfterExternBlock: true | ||
| BeforeCatch: true | ||
| BeforeElse: true | ||
| IndentBraces: false | ||
| SplitEmptyFunction: true | ||
| SplitEmptyRecord: true | ||
| SplitEmptyNamespace: true | ||
| # | ||
| ### BinPack... args will all be on the same line | ||
| BinPackArguments: true | ||
| BinPackParameters: true | ||
| # | ||
| # from LLVM style: | ||
| ###TODO We use only 80 columns! | ||
| ColumnLimit: 80 | ||
| # | ||
| #default: AllowAllParametersOfDeclarationOnNextLine: false | ||
| # AlwaysBreakTemplateDeclarations: MultiLine | ||
| #FIXME BreakBeforeInheritanceComma: false | ||
| BreakBeforeInheritanceComma: true | ||
| BreakConstructorInitializers: BeforeColon | ||
| BreakInheritanceList: BeforeColon | ||
| # PenaltyReturnTypeOnItsOwnLine: 60 | ||
| # SpaceAfterTemplateKeyword: true | ||
| # | ||
| IncludeBlocks: Regroup | ||
| IncludeCategories: | ||
| - Regex: '^("asio.*|<asio.*>)' | ||
| Priority: 2 | ||
| - Regex: '^<(Poco|gsl|doctest|zmqpp|boost|gtest|gmock|fmt|json|openssl)/' | ||
| Priority: 3 | ||
| - Regex: '<[[:alnum:].]+>' | ||
| Priority: 4 | ||
| # all other headers first! | ||
| - Regex: '.*' | ||
| Priority: 1 | ||
| IncludeIsMainRegex: '(_test)?$' | ||
| # | ||
| --- | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| --- | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a CMake related change.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above! |
||
| Checks: "-*,\ | ||
| cert-*,\ | ||
| misc-*,\ | ||
| modernize-*,\ | ||
| -modernize-raw-string-literal,\ | ||
| -modernize-return-braced-init-list,\ | ||
| -modernize-use-auto,\ | ||
| -modernize-use-equals-default,\ | ||
| -modernize-use-equals-delete,\ | ||
| -modernize-use-default-member-init,\ | ||
| -modernize-use-nullptr,\ | ||
| -modernize-use-using,\ | ||
| performance-*,\ | ||
| readability-*,\ | ||
| -readability-named-parameter,\ | ||
| -readability-braces-around-statements,\ | ||
| -readability-implicit-bool-conversion,\ | ||
| -readability-else-after-return,\ | ||
| cppcoreguidelines-*,\ | ||
| -cppcoreguidelines-owning-memory,\ | ||
| -cppcoreguidelines-special-member-functions,\ | ||
| -cppcoreguidelines-pro-*\ | ||
| " | ||
| HeaderFilterRegex: '.*' | ||
| ... | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,240 @@ | ||
| # Minimum supported CMake version | ||
| cmake_minimum_required(VERSION 3.13 FATAL_ERROR) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will change it to |
||
|
|
||
| if(CONAN_COMPILER) | ||
|
|
||
| # ========================================================== | ||
| project(asio VERSION ${CONAN_PACKAGE_VERSION} LANGUAGES CXX) | ||
| # ========================================================== | ||
|
|
||
| include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake) | ||
| conan_basic_setup(TARGETS) # this set CONAN_TARGETS | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the |
||
|
|
||
| else() | ||
|
|
||
| # use ccache and clang++ if found | ||
| find_program(CCACHE_EXECUTABLE "ccache" HINTS /usr/local/bin /opt/local/bin) | ||
| if(CCACHE_EXECUTABLE) | ||
| message(STATUS "use ccache") | ||
| set(CMAKE_CXX_COMPILER_LAUNCHER "${CCACHE_EXECUTABLE}" CACHE PATH "ccache" FORCE) | ||
| set(CMAKE_C_COMPILER_LAUNCHER "${CCACHE_EXECUTABLE}" CACHE PATH "ccache" FORCE) | ||
| endif() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Put this in a superbuild CML, not the project CML. ALternatively, just let people decide whether they want to pass this in from the CLI.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why waste time with compiling? |
||
|
|
||
| # ======================================== | ||
| project(asio VERSION 1.14.1 LANGUAGES CXX) | ||
| # ======================================== | ||
|
|
||
| endif() | ||
|
|
||
| #---------------------------------------------------------- | ||
| # Compiler config | ||
| #---------------------------------------------------------- | ||
| if(NOT DEFINED CMAKE_CXX_STANDARD) | ||
| set(CMAKE_CXX_STANDARD 17) | ||
| set(CMAKE_CXX_STANDARD_REQUIRED ON) | ||
| set(CMAKE_CXX_EXTENSIONS OFF) | ||
| endif() | ||
| message(STATUS "USING: CMAKE_CXX_STANDARD=${CMAKE_CXX_STANDARD}") | ||
| message(STATUS "USING: CMAKE_CXX_STANDARD_REQUIRED=${CMAKE_CXX_STANDARD_REQUIRED}") | ||
| message(STATUS "USING: CMAKE_CXX_EXTENSIONS=${CMAKE_CXX_EXTENSIONS}") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use |
||
|
|
||
|
|
||
| ########################################################### | ||
| option(ASIO_STANDALONE "Build asio without boost" ON) | ||
| ########################################################### | ||
| if(ASIO_STANDALONE) | ||
| option(COMPILER_WARNINGS_ARE_ERRORS "To be pedantic! ;-)" ON) | ||
| if(COMPILER_WARNINGS_ARE_ERRORS) | ||
| if(MSVC) | ||
| # warning level 4 and all warnings as errors | ||
| add_compile_options(/W4 /WX) | ||
| else() | ||
| # lots of warnings and all warnings as errors | ||
| add_compile_options(-Wall -Wextra -Wpedantic -Werror) | ||
| endif() | ||
| endif() | ||
| else() | ||
| unset(COMPILER_WARNINGS_ARE_ERRORS CACHE) | ||
| endif() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has nothing to do in the project CML. Warnings are not a build nor a usage requirement. These should go in the test CML. |
||
| ########################################################### | ||
|
|
||
|
|
||
| # ========================================================= | ||
| # ASIO Options | ||
| #========================================================== | ||
| option(ASIO_NO_DEPRECATED "Build asio without deprecated interfaces" ON) | ||
| option(ASIO_BUILD_TESTS "Build asio unittest too" OFF) | ||
| option(ASIO_BUILD_EXAMPLES "Build asio examples too" OFF) | ||
| option(ASIO_ENABLE_HANDLER_TRACKING "enable handler tracking" OFF) | ||
| option(ASIO_DISABLE_KQUEUE "disable kqueue" OFF) | ||
| option(ASIO_DISABLE_EPOLL "disable epoll" OFF) | ||
| option(ASIO_USE_CLANG_TIDY "use clang-tidy if found" OFF) | ||
|
|
||
|
|
||
| # | ||
| # Where to put all the RUNTIME targets when built. This variable is used | ||
| # to initialize the RUNTIME_OUTPUT_DIRECTORY property on all the targets. | ||
| # | ||
| set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/bin) | ||
| set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib) | ||
| set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you forcing a build folder layout? This isn't portable.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is absolute portable, it is the |
||
|
|
||
|
|
||
| set(CPPdefinitions) | ||
| set(EXTRA_LIBS) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Everything related to these variables should just go in the respective |
||
|
|
||
| if(NOT CONAN_COMPILER AND WIN32) | ||
| list(APPEND CPPdefinitions _WIN32_WINNT=0x0501 __USE_W32_SOCKETS | ||
| ASIO_HAS_VARIADIC_TEMPLATES ASIO_DISABLE_CONSTEXPR | ||
| ) | ||
| set(EXTRA_LIBS wsock32 ws2_32) | ||
| else() | ||
| set(EXTRA_LIBS ${CONAN_TARGETS}) | ||
| endif() | ||
|
|
||
| if(NOT CONAN_COMPILER AND ${CMAKE_HOST_SYSTEM_NAME} MATCHES "MINGW.*") | ||
| list(APPEND CPPdefinitions _WIN32_WINNT=0x0501 __USE_W32_SOCKETS) | ||
| endif() | ||
|
|
||
|
|
||
| if(ASIO_NO_DEPRECATED) | ||
| message(STATUS "disable deprected interfaces") | ||
| list(APPEND CPPdefinitions ASIO_NO_DEPRECATED) | ||
| endif() | ||
|
|
||
| if(ASIO_ENABLE_HANDLER_TRACKING) | ||
| message(STATUS "enable handler tracking") | ||
| list(APPEND CPPdefinitions ASIO_ENABLE_HANDLER_TRACKING) | ||
| endif() | ||
|
|
||
| if(ASIO_DISABLE_KQUEUE) | ||
| message(STATUS "disble queue") | ||
| list(APPEND CPPdefinitions ASIO_DISABLE_KQUEUE) | ||
| endif() | ||
|
|
||
| if(ASIO_DISABLE_EPOLL) | ||
| message(STATUS "disable epoll") | ||
| list(APPEND CPPdefinitions ASIO_DISABLE_EPOLL) | ||
| endif() | ||
|
|
||
| if(ASIO_USE_CLANG_TIDY) | ||
| message(STATUS "use clang-tidy") | ||
| find_program(CMAKE_CXX_CLANG_TIDY | ||
| NAMES clang-tidy clang-tidy-8 clang-tidy-9 clang-tidy-7 | ||
| HINTS /usr/local/bin /opt/local/bin | ||
| ) | ||
| else() | ||
| unset(CMAKE_CXX_CLANG_TIDY CACHE) | ||
| endif() | ||
|
|
||
| if(NOT CONAN_COMPILER) | ||
| if(NOT ASIO_STANDALONE) | ||
| ##################################################################### | ||
| # use boost if found | ||
| option(Boost_USE_MULTITHREADED "Set to OFF to use the non-multithreaded" ON) | ||
| option(Boost_DEBUG "Set to ON to enable debug output from FindBoost." OFF) | ||
| option(Boost_DETAILED_FAILURE_MSG "Set to ON to get detailed information" ON) | ||
| # Set Boost_NO_BOOST_CMAKE to ON to disable the search for boost-cmake. | ||
| set(Boost_NO_BOOST_CMAKE ON) | ||
| set(BOOST_ROOT "/opt/local" CACHE PATH "path to the boost lib") | ||
| set(Boost_USE_STATIC_LIBS OFF) | ||
| if(WIN32) | ||
| set(Boost_USE_STATIC_RUNTIME OFF) | ||
| endif() | ||
| find_package(Boost 1.67 | ||
| COMPONENTS atomic chrono coroutine context thread system | ||
| ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of this should just be
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure why should it be REQUIRED? |
||
| ##################################################################### | ||
|
|
||
| if(Boost_FOUND) | ||
| include_directories(${Boost_INCLUDE_DIRS}) | ||
| link_directories(${Boost_LIBRARY_DIRS}) | ||
| list(APPEND CPPdefinitions BOOST_ALL_NO_LIB=1 | ||
| BOOST_CHRONO_DONT_PROVIDE_HYBRID_ERROR_HANDLING | ||
| ) | ||
| list(APPEND EXTRA_LIBS ${Boost_LIBRARIES}) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Drop the variables and use the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Than we need Boost v1.71 at least, not available on debian stable! |
||
| endif() | ||
| ##################################################################### | ||
| endif() | ||
|
|
||
|
|
||
| ####################################################################### | ||
| # use openssl if found | ||
| set(OPENSSL_ROOT_DIR "/usr" CACHE PATH "path to the openssl lib") | ||
| find_package(OpenSSL HINTS ${OPENSSL_ROOT_DIR} /opt/local) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the hint? You are also missing
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NO, it is optional! |
||
| ####################################################################### | ||
|
|
||
| if(OpenSSL_FOUND) | ||
| list(APPEND EXTRA_LIBS ${openSSL_LIBRARIES}) | ||
| message(STATUS "HAVE_OPENSSL") | ||
| list(APPEND CPPdefinitions HAVE_OPENSSL) | ||
| include_directories(BEFORE ${openSSL_INCLUDE_DIR}) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above. Just use targets.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this patch is very old. On the other side, |
||
| # else() | ||
| # find_library(OpenSSL_LIBRARY ssl HINTS ${OpenSSL_ROOT_DIR}/lib) | ||
| # find_library(CRYPTO_LIBRARY crypto HINTS ${OpenSSL_ROOT_DIR}/lib) | ||
| # if(OpenSSL_LIBRARY AND CRYPTO_LIBRARY) | ||
| # set(OpenSSL_FOUND ON) | ||
| # message(STATUS "USE CRYPTO_LIBRARY") | ||
| # list(APPEND CPPdefinitions HAVE_OPENSSL) | ||
| # list(APPEND EXTRA_LIBS ${OpenSSL_LIBRARY} ${CRYPTO_LIBRARY}) | ||
| # include_directories(BEFORE ${OpenSSL_ROOT_DIR}/include) | ||
| # endif() | ||
| endif() | ||
| ####################################################################### | ||
| else() | ||
| message(STATUS "ASIO USE CONAN_TARGETS") | ||
| if(CONAN_LIBS_OPENSSL) | ||
| list(APPEND CPPdefinitions HAVE_OPENSSL) | ||
| message(STATUS "have CONAN_LIBS_OPENSSL") | ||
| endif() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just unnecessary noise. The user already knows what's on his system. Why tell him?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just unnecessary noise. The user already knows what's on his system. Why tell him? |
||
| endif() | ||
|
|
||
|
|
||
| if(UNIX) | ||
| find_library(PTHREAD_LIBRARY pthread REQUIRED) | ||
| message(STATUS "USE ${PTHREAD_LIBRARY}") | ||
| list(APPEND EXTRA_LIBS ${PTHREAD_LIBRARY}) | ||
| endif() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The conditional is unnecessary, just use
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes |
||
|
|
||
| message(STATUS "CPPdefinitions:${CPPdefinitions}") | ||
|
|
||
|
|
||
| # Installation configuration | ||
| include(GNUInstallDirs) | ||
| set(ConfigPackageLocation ${CMAKE_INSTALL_LIBDIR}/cmake/asio) | ||
|
|
||
| # Interface library | ||
| add_subdirectory(include) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick, but it's completely unnecessary to break things out like this. I shouldn't have to dive through a bunch of files just to see all the usage and build requirements. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick, but it's completely unnecessary to break things out like this. I shouldn't have to dive through a bunch of files just to see all the usage and build requirements. |
||
|
|
||
|
|
||
| if($ENV{CONAN_RUN_TESTS}) | ||
| set(ASIO_BUILD_TESTS ON CACHE BOOL "" FORCE) | ||
| message(STATUS "ASIO CONAN_RUN_TESTS") | ||
| endif() | ||
|
|
||
|
|
||
| if(ASIO_BUILD_EXAMPLES OR ASIO_BUILD_TESTS) | ||
| # enable ctest | ||
| enable_testing() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is bad, create a separate project CML for tests. |
||
|
|
||
| # Sources: examples, tests | ||
| add_subdirectory(src) | ||
| endif() | ||
|
|
||
|
|
||
| # Installation | ||
| include(CMakePackageConfigHelpers) | ||
| configure_package_config_file(asio-config.cmake.in | ||
| ${CMAKE_CURRENT_BINARY_DIR}/asio-config.cmake | ||
| INSTALL_DESTINATION ${ConfigPackageLocation}) | ||
| write_basic_package_version_file( | ||
| ${CMAKE_CURRENT_BINARY_DIR}/asio-config-version.cmake | ||
| VERSION ${asio_VERSION} | ||
| COMPATIBILITY SameMajorVersion) | ||
| install(FILES ${CMAKE_CURRENT_BINARY_DIR}/asio-config-version.cmake | ||
| ${CMAKE_CURRENT_BINARY_DIR}/asio-config.cmake | ||
| DESTINATION ${ConfigPackageLocation}) | ||
| install(EXPORT asio-targets | ||
| DESTINATION ${ConfigPackageLocation} | ||
| NAMESPACE asio::) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are many things wrong with these install rules:
And some nitpicks:
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| @PACKAGE_INIT@ | ||
|
|
||
| include(${CMAKE_CURRENT_LIST_DIR}/asio-targets.cmake) | ||
| check_required_components(asio) | ||
|
|
||
| include(CMakeFindDependencyMacro) | ||
| find_dependency(OpenSSL QUIET) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is wrong, you must find dependencies before you make the targets available. The purpose of include(CMakeFindDependencyMacro)
find_dependency(OpenSSL)
include("${CMAKE_CURRENT_LIST_DIR}/asioTargets.cmake")See my other comment about htis file not requiring configuration. |
||
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.
While I approve, this PR is about CMake. This is not a CMake related change.
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.
That is true, but I love it!