From 82dc7af9b67ff2b554614d6f28275d6673739734 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Fri, 20 Dec 2024 17:46:53 -0800 Subject: [PATCH] - 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