From ecc76148c9e11da416d436de3b34f5ec7acdbce3 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Thu, 19 Dec 2024 13:09:17 -0800 Subject: [PATCH 1/6] Install CMake modules in a more standard way. --- CMakeLists.txt | 6 +++--- cmake/aws-c-common-config.cmake | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6d14b0241..bdff043cc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -278,12 +278,12 @@ else() endif() install(EXPORT "${PROJECT_NAME}-targets" - DESTINATION "${LIBRARY_DIRECTORY}/${PROJECT_NAME}/cmake/${TARGET_DIR}" + DESTINATION "${LIBRARY_DIRECTORY}/cmake/${PROJECT_NAME}/${TARGET_DIR}" NAMESPACE AWS:: COMPONENT Development) install(FILES "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}-config.cmake" - DESTINATION "${LIBRARY_DIRECTORY}/${PROJECT_NAME}/cmake" + DESTINATION "${LIBRARY_DIRECTORY}/cmake/${PROJECT_NAME}" COMPONENT Development) list(APPEND EXPORT_MODULES @@ -300,7 +300,7 @@ list(APPEND EXPORT_MODULES ) install(FILES ${EXPORT_MODULES} - DESTINATION "${LIBRARY_DIRECTORY}/cmake" + DESTINATION "${LIBRARY_DIRECTORY}/cmake/${PROJECT_NAME}/modules" COMPONENT Development) # This should come last, to ensure all variables defined by cmake will be available for export diff --git a/cmake/aws-c-common-config.cmake b/cmake/aws-c-common-config.cmake index 094ad116d..2964427f4 100644 --- a/cmake/aws-c-common-config.cmake +++ b/cmake/aws-c-common-config.cmake @@ -1,3 +1,6 @@ +# make installed modules (e.g. AwsCFlags.cmake) available to dependencies +list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR}/modules) + set(THREADS_PREFER_PTHREAD_FLAG ON) if(WIN32 OR UNIX OR APPLE) From c720814e48a99370c3c1a2928bb82d9ce66c9f05 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Thu, 19 Dec 2024 16:15:54 -0800 Subject: [PATCH 2/6] respect custom cmake install path for bin/lib/include --- CMakeLists.txt | 4 ++-- bin/system_info/CMakeLists.txt | 2 -- cmake/AwsSharedLibSetup.cmake | 22 +++++++++------------- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bdff043cc..2dbddad2b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -247,7 +247,7 @@ foreach(HEADER_SRCPATH IN ITEMS ${AWS_COMMON_HEADERS} ${AWS_COMMON_OS_HEADERS} $ unset(HEADER_DSTDIR) - foreach(POTENTIAL_PREFIX IN ITEMS ${GENERATED_ROOT_DIR} ${CMAKE_CURRENT_SOURCE_DIR}) + foreach(POTENTIAL_PREFIX IN ITEMS "${GENERATED_INCLUDE_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}/include") string(LENGTH ${POTENTIAL_PREFIX} _prefixlen) string(SUBSTRING ${HEADER_DIR} 0 ${_prefixlen} _actual_prefix) if(${_actual_prefix} STREQUAL ${POTENTIAL_PREFIX}) @@ -261,7 +261,7 @@ foreach(HEADER_SRCPATH IN ITEMS ${AWS_COMMON_HEADERS} ${AWS_COMMON_OS_HEADERS} $ endif() install(FILES ${HEADER_SRCPATH} - DESTINATION ${HEADER_DSTDIR} + DESTINATION "${INCLUDE_DIRECTORY}/${HEADER_DSTDIR}" COMPONENT Development) endforeach() diff --git a/bin/system_info/CMakeLists.txt b/bin/system_info/CMakeLists.txt index cb4ca0081..79f0c0acd 100644 --- a/bin/system_info/CMakeLists.txt +++ b/bin/system_info/CMakeLists.txt @@ -1,7 +1,5 @@ project(print-sys-info C) -list(APPEND CMAKE_MODULE_PATH "${CMAKE_INSTALL_PREFIX}/lib/cmake") - file(GLOB SI_SRC "*.c" ) diff --git a/cmake/AwsSharedLibSetup.cmake b/cmake/AwsSharedLibSetup.cmake index 856b1c8a5..404a373ee 100644 --- a/cmake/AwsSharedLibSetup.cmake +++ b/cmake/AwsSharedLibSetup.cmake @@ -1,22 +1,18 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0. -set(LIBRARY_DIRECTORY lib) -set(RUNTIME_DIRECTORY bin) -# Set the default lib installation path on GNU systems with GNUInstallDirs -if (UNIX AND NOT APPLE) - include(GNUInstallDirs) - set(LIBRARY_DIRECTORY ${CMAKE_INSTALL_LIBDIR}) - set(RUNTIME_DIRECTORY ${CMAKE_INSTALL_BINDIR}) +# NOTE: GNUInstallDirs is also fine for Windows and Mac. It sets reasonable defaults like "lib" and "bin" +include(GNUInstallDirs) +set(LIBRARY_DIRECTORY ${CMAKE_INSTALL_LIBDIR}) +set(RUNTIME_DIRECTORY ${CMAKE_INSTALL_BINDIR}) +set(INCLUDE_DIRECTORY ${CMAKE_INSTALL_INCLUDEDIR}) - # this is the absolute dumbest thing in the world, but find_package won't work without it - # also I verified this is correctly NOT "lib64" when CMAKE_C_FLAGS includes "-m32" - if (${LIBRARY_DIRECTORY} STREQUAL "lib64") - set(FIND_LIBRARY_USE_LIB64_PATHS true) - endif() +# this is the absolute dumbest thing in the world, but find_package won't work without it +# also I verified this is correctly NOT "lib64" when CMAKE_C_FLAGS includes "-m32" +if (${LIBRARY_DIRECTORY} STREQUAL "lib64") + set(FIND_LIBRARY_USE_LIB64_PATHS true) endif() - function(aws_prepare_shared_lib_exports target) if (BUILD_SHARED_LIBS) install(TARGETS ${target} From 271d2489303b00b78100ae8b2d06d752578576b7 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Fri, 20 Dec 2024 10:32:29 -0800 Subject: [PATCH 3/6] Fix CMake Deprecation Warning, by declaring project is known to be compatible with latest versions too --- AWSCRTAndroidTestRunner/app/src/main/cpp/CMakeLists.txt | 2 +- CMakeLists.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/AWSCRTAndroidTestRunner/app/src/main/cpp/CMakeLists.txt b/AWSCRTAndroidTestRunner/app/src/main/cpp/CMakeLists.txt index 9a91baba4..d617d7804 100644 --- a/AWSCRTAndroidTestRunner/app/src/main/cpp/CMakeLists.txt +++ b/AWSCRTAndroidTestRunner/app/src/main/cpp/CMakeLists.txt @@ -3,7 +3,7 @@ # Sets the minimum version of CMake required to build the native library. -cmake_minimum_required(VERSION 3.9) +cmake_minimum_required(VERSION 3.9...3.31) # AWS lib set(path_to_common "${CMAKE_CURRENT_LIST_DIR}/../../../../..") diff --git a/CMakeLists.txt b/CMakeLists.txt index 2dbddad2b..b85c10314 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ # As of October 2024, we picked 3.9 as our version because internally we still support RHEL5 and AL2012, and CMake 3.9 # was the latest version available on those platforms. -cmake_minimum_required(VERSION 3.9) +cmake_minimum_required(VERSION 3.9...3.31) option(ALLOW_CROSS_COMPILED_TESTS "Allow tests to be compiled via cross compile, for use with qemu" OFF) project(aws-c-common LANGUAGES C VERSION 0.1.0) From 2b3320d43bd21286d7a2467096a66cd70df6160c Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Fri, 20 Dec 2024 11:59:36 -0800 Subject: [PATCH 4/6] Stop using OLD behavior for CMake policy CMP0077. This eliminates a CMake Deprecation Warning. The comment says we added this to "Enable options to get their values from normal variables" But reading the docs: https://cmake.org/cmake/help/latest/policy/CMP0077.html it seems like the NEW behavior is what lets options get their values from normal variables --- CMakeLists.txt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b85c10314..2fcb5aaef 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,10 +10,6 @@ project(aws-c-common LANGUAGES C VERSION 0.1.0) message(STATUS "CMake ${CMAKE_VERSION}") -if (POLICY CMP0077) - cmake_policy(SET CMP0077 OLD) # Enable options to get their values from normal variables -endif() - list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") include(AwsCFlags) include(AwsCheckHeaders) From 82dc7af9b67ff2b554614d6f28275d6673739734 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Fri, 20 Dec 2024 17:46:53 -0800 Subject: [PATCH 5/6] - Just use CMAKE_INSTALL_LIBDIR etc, instead of making custom variables like LIBRARY_DIRECTORY. Since every platform can do `include(GnuInstallDirs)`, it's fine to rely on the variables it defines, so let's just use them explicitly. - Explicitly include(GNUInstallDirs) in every CMakeLists.txt that uses the vars it defines (e.g. CMAKE_INSTALL_LIBDIR). I could get away omitting this include, since the AwsSharedLibsSetup.cmake helper script also pulls it in, but it seems like good practice. - Remove custom code that was setting FIND_LIBRARY_USE_LIB64_PATHS. In the PR that introduced this https://github.com/awslabs/aws-c-common/pull/226 the reviewer called out that this probably wasn't necessary. ChatGPT agrees. CMake is supposed to figure this out automatically. And if it's not working, then it's a weird cross-compile situation where the person building should be setting this to fix things. It shouldn't be up to every library on earth to do this hack. So, this PR started with me thinking that, if we're getting all the shared scripts via `find_package(aws-c-common)`, and the shared scripts are where FIND_LIBRARY_USE_LIB64_PATHS gets customized, but we need to customize FIND_LIBRARY_USE_LIB64_PATHS before we can do `find_package(aws-c-common)`, then ALL projects need to copy/paste the code that customizes FIND_LIBRARY_USE_LIB64_PATHS before doing `find_package(aws-c-common)`. So I set down that path. Then when writing the commit message I was like ... wait a minute this is ridiculous. So did some more research, and learned the FIND_LIBRARY_USE_LIB64_PATHS stuff was probably unnecessary. --- CMakeLists.txt | 9 +++++---- cmake/AwsSharedLibSetup.cmake | 20 +++++--------------- cmake/CPackConfig.cmake | 2 +- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2fcb5aaef..b6d4b13d1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,6 +19,7 @@ include(AwsSanitizers) include(AwsThreadAffinity) include(AwsThreadName) include(CTest) +include(GNUInstallDirs) set(GENERATED_ROOT_DIR "${CMAKE_CURRENT_BINARY_DIR}/generated") set(GENERATED_INCLUDE_DIR "${GENERATED_ROOT_DIR}/include") @@ -257,7 +258,7 @@ foreach(HEADER_SRCPATH IN ITEMS ${AWS_COMMON_HEADERS} ${AWS_COMMON_OS_HEADERS} $ endif() install(FILES ${HEADER_SRCPATH} - DESTINATION "${INCLUDE_DIRECTORY}/${HEADER_DSTDIR}" + DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/${HEADER_DSTDIR}" COMPONENT Development) endforeach() @@ -274,12 +275,12 @@ else() endif() install(EXPORT "${PROJECT_NAME}-targets" - DESTINATION "${LIBRARY_DIRECTORY}/cmake/${PROJECT_NAME}/${TARGET_DIR}" + DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}/${TARGET_DIR}" NAMESPACE AWS:: COMPONENT Development) install(FILES "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}-config.cmake" - DESTINATION "${LIBRARY_DIRECTORY}/cmake/${PROJECT_NAME}" + DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}" COMPONENT Development) list(APPEND EXPORT_MODULES @@ -296,7 +297,7 @@ list(APPEND EXPORT_MODULES ) install(FILES ${EXPORT_MODULES} - DESTINATION "${LIBRARY_DIRECTORY}/cmake/${PROJECT_NAME}/modules" + DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}/modules" COMPONENT Development) # This should come last, to ensure all variables defined by cmake will be available for export diff --git a/cmake/AwsSharedLibSetup.cmake b/cmake/AwsSharedLibSetup.cmake index 404a373ee..0700a7b1d 100644 --- a/cmake/AwsSharedLibSetup.cmake +++ b/cmake/AwsSharedLibSetup.cmake @@ -1,42 +1,32 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0. -# NOTE: GNUInstallDirs is also fine for Windows and Mac. It sets reasonable defaults like "lib" and "bin" include(GNUInstallDirs) -set(LIBRARY_DIRECTORY ${CMAKE_INSTALL_LIBDIR}) -set(RUNTIME_DIRECTORY ${CMAKE_INSTALL_BINDIR}) -set(INCLUDE_DIRECTORY ${CMAKE_INSTALL_INCLUDEDIR}) - -# this is the absolute dumbest thing in the world, but find_package won't work without it -# also I verified this is correctly NOT "lib64" when CMAKE_C_FLAGS includes "-m32" -if (${LIBRARY_DIRECTORY} STREQUAL "lib64") - set(FIND_LIBRARY_USE_LIB64_PATHS true) -endif() function(aws_prepare_shared_lib_exports target) if (BUILD_SHARED_LIBS) install(TARGETS ${target} EXPORT ${target}-targets ARCHIVE - DESTINATION ${LIBRARY_DIRECTORY} + DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT Development LIBRARY - DESTINATION ${LIBRARY_DIRECTORY} + DESTINATION ${CMAKE_INSTALL_LIBDIR} NAMELINK_SKIP COMPONENT Runtime RUNTIME - DESTINATION ${RUNTIME_DIRECTORY} + DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT Runtime) install(TARGETS ${target} EXPORT ${target}-targets LIBRARY - DESTINATION ${LIBRARY_DIRECTORY} + DESTINATION ${CMAKE_INSTALL_LIBDIR} NAMELINK_ONLY COMPONENT Development) else() install(TARGETS ${target} EXPORT ${target}-targets - ARCHIVE DESTINATION ${LIBRARY_DIRECTORY} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT Development) endif() endfunction() diff --git a/cmake/CPackConfig.cmake b/cmake/CPackConfig.cmake index 2f9d0cc4a..fd076c4f4 100644 --- a/cmake/CPackConfig.cmake +++ b/cmake/CPackConfig.cmake @@ -62,7 +62,7 @@ endif() # By default, we'll try to claim the cmake directory under the library directory # and the aws include directory. We have to share both of these set(CPACK_RPM_EXCLUDE_FROM_AUTO_FILELIST_ADDITION - /usr/${LIBRARY_DIRECTORY}/cmake + /usr/${CMAKE_INSTALL_LIBDIR}/cmake /usr/include/aws) # Include CPack, which generates the package target From 4f30c775e66e9f9e6901d8a1e5fe440e98fafd94 Mon Sep 17 00:00:00 2001 From: Joseph Klix Date: Thu, 2 Jan 2025 13:59:22 -0800 Subject: [PATCH 6/6] change PR template to ask for clearer wording (#1177) --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index ab40d21d7..3482d468d 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,4 +1,4 @@ -*Issue #, if available:* +*Issue #, and/or reason for changes (REQUIRED):* *Description of changes:*