awslabs / aws-c-event-stream

C99 implementation of the vnd.amazon.eventstream content-type.
Apache License 2.0
33 stars 46 forks source link

AWS C/C++ packages use non-standard cmake module paths breaking most distributions #15

Open glaubitz opened 5 years ago

glaubitz commented 5 years ago

I am currently packaging aws-c-common, aws-checkums and aws-c-event-stream for openSUSE as these packages are now required for the aws-sdk-cpp which is already part of openSUSE and SLE.

While packaging, I have run into the problem that the cmake configuration in aws-c-event-stream couldn't find AwsCFlags and AwsSanitizers:

[    5s] CMake Error at CMakeLists.txt:17 (include):
[    5s]   include could not find load file:
[    5s] 
[    5s]     AwsCFlags
[    5s] 
[    5s] 
[    5s] CMake Error at CMakeLists.txt:18 (include):
[    5s]   include could not find load file:
[    5s] 
[    5s]     AwsSanitizers
[    5s] 
[    5s] 
[    5s] CMake Error at CMakeLists.txt:54 (aws_set_common_properties):
[    5s]   Unknown CMake command "aws_set_common_properties".
[    5s] 
[    5s] 
[    5s] -- Configuring incomplete, errors occurred!
[    5s] See also "/home/abuild/rpmbuild/BUILD/aws-c-event-stream-0.1.0/build/CMakeFiles/CMakeOutput.log".
[    5s] error: Bad exit status from /var/tmp/rpm-tmp.QpHCox (%build)

A simple workaround fixes the problem for me:

diff -Nru aws-c-event-stream-0.1.0.orig/CMakeLists.txt aws-c-event-stream-0.1.0/CMakeLists.txt
--- aws-c-event-stream-0.1.0.orig/CMakeLists.txt        2018-12-18 04:15:43.000000000 +0100
+++ aws-c-event-stream-0.1.0/CMakeLists.txt     2019-02-08 14:56:12.388040665 +0100
@@ -13,7 +13,7 @@
 cmake_minimum_required (VERSION 3.1)
 project (aws-c-event-stream C)

-list(APPEND CMAKE_MODULE_PATH "${CMAKE_INSTALL_PREFIX}/lib/cmake")
+list(APPEND CMAKE_MODULE_PATH "${CMAKE_INSTALL_PREFIX}/lib64/cmake")
 include(AwsCFlags)
 include(AwsSanitizers)
 include(CheckCCompilerFlag)

The proper fix is certainly to use find_library() instead of include() in this case.

madebr commented 4 years ago

Imho, the proper fix would be to only do find_package(aws-c-common REQUIRED). The aws-c-common-config.cmake file should modify the CMAKE_MODULE_PATH accordingly. The proper fix is thus in aws-c-common project. Consumers of aws-c-common will then be able to include the AwsCFlags, AwsSharedLibSetup, AwsFeatureTests, AwsSanitizers and AwsSIMD modules.

Lectem commented 4 years ago

I'm having the same issue here while trying to build on amazon linux 2.

glaubitz commented 3 years ago

FWIW, this issue affects many C/C++ AWS packages.

We currently ship these libraries in openSUSE and they all needed to have their cmake paths patched:

I also verified on the cmake paths on Debian/Ubuntu and Fedora/RHEL, they also follow the same directory hierarchy as openSUSE, i.e. $LIBDIR/cmake/$PACKAGE.

Using the proper cmake paths also means that this particular hack that I have found in every CMakeLists.txt of all AWS packages so far is no longer necessary:

# This is required in order to append /lib/cmake to each element in CMAKE_PREFIX_PATH
set(AWS_MODULE_DIR "/${CMAKE_INSTALL_LIBDIR}/cmake")
string(REPLACE ";" "${AWS_MODULE_DIR};" AWS_MODULE_PATH "${CMAKE_PREFIX_PATH}${AWS_MODULE_DIR}")
# Append that generated list to the module search path
list(APPEND CMAKE_MODULE_PATH ${AWS_MODULE_PATH})

I'm currently using this patch for all AWS packages and it fixes the cmake path issues for me:

diff -Nru aws-c-event-stream-0.1.6.orig/CMakeLists.txt aws-c-event-stream-0.1.6/CMakeLists.txt
--- aws-c-event-stream-0.1.6.orig/CMakeLists.txt        2020-07-24 02:06:45.000000000 +0200
+++ aws-c-event-stream-0.1.6/CMakeLists.txt     2020-08-25 13:17:04.469790864 +0200
@@ -11,17 +11,11 @@
     file(TO_CMAKE_PATH "${CMAKE_INSTALL_PREFIX}" CMAKE_INSTALL_PREFIX)
 endif()

-if (UNIX AND NOT APPLE)
-    include(GNUInstallDirs)
-elseif(NOT DEFINED CMAKE_INSTALL_LIBDIR)
-    set(CMAKE_INSTALL_LIBDIR "lib")
-endif()
+find_package(aws-c-common REQUIRED)
+find_package(aws-checksums REQUIRED)
+set(CMAKE_MODULE_PATH ${aws-c-common_DIR})

-# This is required in order to append /lib/cmake to each element in CMAKE_PREFIX_PATH
-set(AWS_MODULE_DIR "/${CMAKE_INSTALL_LIBDIR}/cmake")
-string(REPLACE ";" "${AWS_MODULE_DIR};" AWS_MODULE_PATH "${CMAKE_PREFIX_PATH}${AWS_MODULE_DIR}")
-# Append that generated list to the module search path
-list(APPEND CMAKE_MODULE_PATH ${AWS_MODULE_PATH})
+include(GNUInstallDirs)

 include(AwsCFlags)
 include(AwsSharedLibSetup)
@@ -92,7 +86,7 @@
 endif()

 install(EXPORT "${PROJECT_NAME}-targets"
-    DESTINATION "${LIBRARY_DIRECTORY}/${PROJECT_NAME}/cmake/${TARGET_DIR}/"
+    DESTINATION "${LIB_INSTALL_DIR}/cmake/${CMAKE_PROJECT_NAME}/${TARGET_DIR}/"
     NAMESPACE AWS::
     COMPONENT Development)

@@ -101,7 +95,7 @@
     @ONLY)

 install(FILES "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}-config.cmake"
-    DESTINATION "${LIBRARY_DIRECTORY}/${PROJECT_NAME}/cmake/"
+    DESTINATION "${LIB_INSTALL_DIR}/cmake/${CMAKE_PROJECT_NAME}"
     COMPONENT Development)

It's actually a change that was recommend to me by the cmake upstream developers.

jmklix commented 9 months ago

Can you make PR with the suggested changes?

glaubitz commented 9 months ago

Can you make PR with the suggested changes?

It should be enough to simply switch all packages to using the cmake default paths as they are used by all Linux distributions. It's usually ${LIB_INSTALL_DIR}/cmake/${CMAKE_PROJECT_NAME} while the AWS C/C++ SDK packages default to ${CMAKE_INSTALL_LIBDIR}/${PROJECT_NAME}/cmake/ which is why cmake won't be able to find AWS cmake modules by default.