From 72d849d4b67c9b54f7e171d677a44fd88be84216 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 15 May 2020 17:07:16 +0200 Subject: [PATCH 1/6] cmake: Align declaration of include directory Align declaration of ./include include directory among libraries, static and shared. Signed-off-by: Ronald Cron --- library/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index f6a186faef..960e232ae2 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -168,10 +168,14 @@ if(USE_STATIC_MBEDTLS_LIBRARY) add_library(${mbedx509_static_target} STATIC ${src_x509}) set_target_properties(${mbedx509_static_target} PROPERTIES OUTPUT_NAME mbedx509) target_link_libraries(${mbedx509_static_target} ${libs} ${mbedcrypto_static_target}) + target_include_directories(${mbedx509_static_target} + PUBLIC ${MBEDTLS_DIR}/include/) add_library(${mbedtls_static_target} STATIC ${src_tls}) set_target_properties(${mbedtls_static_target} PROPERTIES OUTPUT_NAME mbedtls) target_link_libraries(${mbedtls_static_target} ${libs} ${mbedx509_static_target}) + target_include_directories(${mbedtls_static_target} + PUBLIC ${MBEDTLS_DIR}/include/) install(TARGETS ${mbedtls_static_target} ${mbedx509_static_target} ${mbedcrypto_static_target} DESTINATION ${LIB_INSTALL_DIR} From 00f5b8cd63dca45d3045e72cb4ead2111ebbbdce Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 25 May 2020 09:39:09 +0200 Subject: [PATCH 2/6] cmake: Compile everest code only if necessary Compile everest code only if MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED is defined in config.h Signed-off-by: Ronald Cron --- 3rdparty/CMakeLists.txt | 6 +++++- 3rdparty/everest/CMakeLists.txt | 20 +++++++------------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/3rdparty/CMakeLists.txt b/3rdparty/CMakeLists.txt index dca4bd76b5..4c5b5599dc 100644 --- a/3rdparty/CMakeLists.txt +++ b/3rdparty/CMakeLists.txt @@ -3,7 +3,11 @@ list (APPEND thirdparty_lib) list (APPEND thirdparty_inc) list (APPEND thirdparty_def) -add_subdirectory(everest) +execute_process(COMMAND ${MBEDTLS_PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/config.py -f ${CMAKE_CURRENT_SOURCE_DIR}/../include/mbedtls/config.h get MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED RESULT_VARIABLE result) + +if(${result} EQUAL 0) + add_subdirectory(everest) +endif() set(thirdparty_src ${thirdparty_src} PARENT_SCOPE) set(thirdparty_lib ${thirdparty_lib} PARENT_SCOPE) diff --git a/3rdparty/everest/CMakeLists.txt b/3rdparty/everest/CMakeLists.txt index c27a8e5ee9..a900e4b634 100644 --- a/3rdparty/everest/CMakeLists.txt +++ b/3rdparty/everest/CMakeLists.txt @@ -10,21 +10,15 @@ set(everest_src list(APPEND everest_inc ${CMAKE_CURRENT_SOURCE_DIR}/include ${CMAKE_CURRENT_SOURCE_DIR}/include/everest ${CMAKE_CURRENT_SOURCE_DIR}/include/everest/kremlib) -execute_process(COMMAND ${MBEDTLS_PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/../../scripts/config.py -f ${CMAKE_CURRENT_SOURCE_DIR}/../../include/mbedtls/config.h get MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED RESULT_VARIABLE result) +if(INSTALL_MBEDTLS_HEADERS) -if(${result} EQUAL 0) + install(DIRECTORY include/everest + DESTINATION include + FILE_PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ + DIRECTORY_PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE + FILES_MATCHING PATTERN "*.h") - if(INSTALL_MBEDTLS_HEADERS) - - install(DIRECTORY include/everest - DESTINATION include - FILE_PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ - DIRECTORY_PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE - FILES_MATCHING PATTERN "*.h") - - endif(INSTALL_MBEDTLS_HEADERS) - -endif() +endif(INSTALL_MBEDTLS_HEADERS) set(thirdparty_src ${thirdparty_src} ${everest_src} PARENT_SCOPE) set(thirdparty_inc ${thirdparty_inc} ${everest_inc} PARENT_SCOPE) From f19f312aa6e7ff6b75f62e07ac9bb1cd6aa10ad3 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 25 May 2020 10:26:37 +0200 Subject: [PATCH 3/6] cmake: Add 3rd party public include directories Add the possibility to distinguish between public and non-public include directories. Public directories are the one to use to access definitions of 3rd party code interfaces. Signed-off-by: Ronald Cron --- 3rdparty/CMakeLists.txt | 2 ++ 3rdparty/everest/CMakeLists.txt | 5 ++++- CMakeLists.txt | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/3rdparty/CMakeLists.txt b/3rdparty/CMakeLists.txt index 4c5b5599dc..18945e52ee 100644 --- a/3rdparty/CMakeLists.txt +++ b/3rdparty/CMakeLists.txt @@ -1,5 +1,6 @@ list (APPEND thirdparty_src) list (APPEND thirdparty_lib) +list (APPEND thirdparty_inc_public) list (APPEND thirdparty_inc) list (APPEND thirdparty_def) @@ -11,5 +12,6 @@ endif() set(thirdparty_src ${thirdparty_src} PARENT_SCOPE) set(thirdparty_lib ${thirdparty_lib} PARENT_SCOPE) +set(thirdparty_inc_public ${thirdparty_inc_public} PARENT_SCOPE) set(thirdparty_inc ${thirdparty_inc} PARENT_SCOPE) set(thirdparty_def ${thirdparty_def} PARENT_SCOPE) diff --git a/3rdparty/everest/CMakeLists.txt b/3rdparty/everest/CMakeLists.txt index a900e4b634..d81d995f1f 100644 --- a/3rdparty/everest/CMakeLists.txt +++ b/3rdparty/everest/CMakeLists.txt @@ -1,4 +1,5 @@ list (APPEND everest_src) +list (APPEND everest_inc_public) list (APPEND everest_inc) list (APPEND everest_def) @@ -8,7 +9,8 @@ set(everest_src ${CMAKE_CURRENT_SOURCE_DIR}/library/Hacl_Curve25519_joined.c ) -list(APPEND everest_inc ${CMAKE_CURRENT_SOURCE_DIR}/include ${CMAKE_CURRENT_SOURCE_DIR}/include/everest ${CMAKE_CURRENT_SOURCE_DIR}/include/everest/kremlib) +list(APPEND everest_inc_public ${CMAKE_CURRENT_SOURCE_DIR}/include) +list(APPEND everest_inc ${CMAKE_CURRENT_SOURCE_DIR}/include/everest ${CMAKE_CURRENT_SOURCE_DIR}/include/everest/kremlib) if(INSTALL_MBEDTLS_HEADERS) @@ -21,5 +23,6 @@ if(INSTALL_MBEDTLS_HEADERS) endif(INSTALL_MBEDTLS_HEADERS) set(thirdparty_src ${thirdparty_src} ${everest_src} PARENT_SCOPE) +set(thirdparty_inc_public ${thirdparty_inc_public} ${everest_inc_public} PARENT_SCOPE) set(thirdparty_inc ${thirdparty_inc} ${everest_inc} PARENT_SCOPE) set(thirdparty_def ${thirdparty_def} ${everest_def} PARENT_SCOPE) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1f675c1ee0..9c3c86ce13 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -219,6 +219,7 @@ endif(ENABLE_ZLIB_SUPPORT) add_subdirectory(include) add_subdirectory(3rdparty) +include_directories(${thirdparty_inc_public}) include_directories(${thirdparty_inc}) list(APPEND libs ${thirdparty_lib}) add_definitions(${thirdparty_def}) From 67d4b555b8e31a93eb6466c77cdd81fc78d613fe Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 22 May 2020 21:59:53 +0200 Subject: [PATCH 4/6] cmake: Limit scope of 3rd party definitions Don't define anymore globally third party include directories and compile definitions. Declare them within the scope of the crypto library target as per the third party source files. Note that targets linking to the crypto library inherit from the third party public include directories. Signed-off-by: Ronald Cron --- CMakeLists.txt | 3 --- library/CMakeLists.txt | 12 ++++++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9c3c86ce13..de50c72aa3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -219,10 +219,7 @@ endif(ENABLE_ZLIB_SUPPORT) add_subdirectory(include) add_subdirectory(3rdparty) -include_directories(${thirdparty_inc_public}) -include_directories(${thirdparty_inc}) list(APPEND libs ${thirdparty_lib}) -add_definitions(${thirdparty_def}) add_subdirectory(library) diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 960e232ae2..0a8b87cc7f 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -163,7 +163,11 @@ if(USE_STATIC_MBEDTLS_LIBRARY) set_target_properties(${mbedcrypto_static_target} PROPERTIES OUTPUT_NAME mbedcrypto) target_link_libraries(${mbedcrypto_static_target} ${libs}) target_include_directories(${mbedcrypto_static_target} - PUBLIC ${MBEDTLS_DIR}/include/) + PUBLIC ${MBEDTLS_DIR}/include/ + PUBLIC ${thirdparty_inc_public} + PRIVATE ${thirdparty_inc}) + target_compile_definitions(${mbedcrypto_static_target} + PRIVATE ${thirdparty_def}) add_library(${mbedx509_static_target} STATIC ${src_x509}) set_target_properties(${mbedx509_static_target} PROPERTIES OUTPUT_NAME mbedx509) @@ -188,7 +192,11 @@ if(USE_SHARED_MBEDTLS_LIBRARY) set_target_properties(mbedcrypto PROPERTIES VERSION 2.22.0 SOVERSION 4) target_link_libraries(mbedcrypto ${libs}) target_include_directories(mbedcrypto - PUBLIC ${MBEDTLS_DIR}/include/) + PUBLIC ${MBEDTLS_DIR}/include/ + PUBLIC ${thirdparty_inc_public} + PRIVATE ${thirdparty_inc}) + target_compile_definitions(mbedcrypto + PRIVATE ${thirdparty_def}) add_library(mbedx509 SHARED ${src_x509}) set_target_properties(mbedx509 PROPERTIES VERSION 2.22.0 SOVERSION 1) From 855274113ac8b49a6d9d72e08d2f06e7d0c17b0a Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 15 May 2020 17:20:47 +0200 Subject: [PATCH 5/6] cmake: Remove global include directories Remove the declaration of ./include and ./library as include directories for all targets. Prefer being more local and declare include directories at the target level using target_include_directories(). Note that there is no need to declare explicitely ./include as an include directory for tests as they inherit it from the "mbed librairies". Signed-off-by: Ronald Cron --- CMakeLists.txt | 3 --- tests/CMakeLists.txt | 10 ++++++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index de50c72aa3..61f149f468 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -205,9 +205,6 @@ else() set(LIB_INSTALL_DIR lib) endif() -include_directories(include/) -include_directories(library/) - if(ENABLE_ZLIB_SUPPORT) find_package(ZLIB) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 39a7a2cd0e..182109b519 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -48,7 +48,10 @@ function(add_test_suite suite_name) add_executable(test_suite_${data_name} test_suite_${data_name}.c $) target_link_libraries(test_suite_${data_name} ${libs}) - target_include_directories(test_suite_${data_name} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include) + target_include_directories(test_suite_${data_name} + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../library) + if(${data_name} MATCHES ${SKIP_TEST_SUITES_REGEX}) message(STATUS "The test suite ${data_name} will not be executed.") else() @@ -68,7 +71,10 @@ endif(MSVC) file(GLOB MBEDTESTS_FILES ${CMAKE_CURRENT_SOURCE_DIR}/src/*.c) add_library(mbedtests OBJECT ${MBEDTESTS_FILES}) -target_include_directories(mbedtests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include) +target_include_directories(mbedtests + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../include + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../library) add_test_suite(aes aes.cbc) add_test_suite(aes aes.cfb) From b1790af64802fa0a2f58cff1c3f2f464a82e3815 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 20 May 2020 15:34:47 +0200 Subject: [PATCH 6/6] cmake: Add include directory policy documentation Signed-off-by: Ronald Cron --- CMakeLists.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 61f149f468..7838525495 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,3 +1,21 @@ +# +# CMake build system design considerations: +# +# - Include directories: +# + Do not define include directories globally using the include_directories +# command but rather at the target level using the +# target_include_directories command. That way, it is easier to guarantee +# that targets are built using the proper list of include directories. +# + Use the PUBLIC and PRIVATE keywords to specifiy the scope of include +# directories. That way, a target linking to a library (using the +# target_link_librairies command) inherits from the library PUBLIC include +# directories and not from the PRIVATE ones. +# + Note: there is currently one remaining include_directories command in the +# CMake files. It is related to ZLIB support which is planned to be removed. +# When the support is removed, the associated include_directories command +# will be removed as well as this note. +# + cmake_minimum_required(VERSION 2.6) if(TEST_CPP) project("mbed TLS" C CXX)