Skip to content

Commit

Permalink
- Just use CMAKE_INSTALL_LIBDIR etc, instead of making custom variabl…
Browse files Browse the repository at this point in the history
…es 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 #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.
  • Loading branch information
graebm committed Dec 21, 2024
1 parent 2b3320d commit 82dc7af
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 20 deletions.
9 changes: 5 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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()

Expand All @@ -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
Expand All @@ -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
Expand Down
20 changes: 5 additions & 15 deletions cmake/AwsSharedLibSetup.cmake
Original file line number Diff line number Diff line change
@@ -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()
Expand Down
2 changes: 1 addition & 1 deletion cmake/CPackConfig.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 82dc7af

Please sign in to comment.