Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
glaubitz opened this issue Feb 8, 2019 · 5 comments · May be fixed by awslabs/aws-c-common#850
Labels
bug We can reproduce the issue and confirmed it is a bug. needs-review This issue or pull request needs review from a core team member. p2 This is a standard priority issue

Comments

@glaubitz
Copy link

glaubitz commented Feb 8, 2019

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.

glaubitz referenced this issue Apr 3, 2019
* Properly handle lib prefix.

* Removed unnecessary message.
@madebr
Copy link

madebr commented Nov 6, 2019

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
Copy link

Lectem commented Feb 4, 2020

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

Lectem added a commit to Lectem/aws-c-common that referenced this issue Feb 5, 2020
Right now aws-c-event-stream needs a lot of boilerplate code to determine the modules installation path while the aws-c-common package should provide it.
This would fix awslabs/aws-c-event-stream#15 simply by moving `find_package(aws-c-common REQUIRED)` before the various module `include`s
Using `configure_package_config_file` makes it easier to have a relocatable package.
Lectem added a commit to Lectem/aws-c-event-stream that referenced this issue Feb 5, 2020
`aws-c-event-stream` should not need to worry nor now how to find the `aws-c-common` CMake modules.
This can be fixed by simply moving up `find_package(aws-c-common REQUIRED)` and making the `aws-c-common` package set the paths correctly.

This has been confirmed to work on amazon linux 2.

This commit requires awslabs/aws-c-common#587 to be merged first.
@glaubitz
Copy link
Author

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:

  • aws-c-cal
  • aws-c-common
  • aws-c-event-stream
  • aws-c-io
  • aws-checksums
  • s2n

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.

@glaubitz glaubitz changed the title Hardwired cmake module path incompatible with 64-bit RPM-based systems AWS C/C++ packages use non-standard cmake module path breaking most distributions Nov 27, 2020
@glaubitz glaubitz changed the title AWS C/C++ packages use non-standard cmake module path breaking most distributions AWS C/C++ packages use non-standard cmake module paths breaking most distributions Nov 27, 2020
r-burns pushed a commit to r-burns/aws-c-common that referenced this issue Oct 8, 2021
Right now aws-c-event-stream needs a lot of boilerplate code to determine the modules installation path while the aws-c-common package should provide it.
This would fix awslabs/aws-c-event-stream#15 simply by moving `find_package(aws-c-common REQUIRED)` before the various module `include`s
Using `configure_package_config_file` makes it easier to have a relocatable package.
r-burns pushed a commit to r-burns/aws-c-common that referenced this issue Oct 8, 2021
Right now aws-c-event-stream needs a lot of boilerplate code to determine the modules installation path while the aws-c-common package should provide it.
This would fix awslabs/aws-c-event-stream#15 simply by moving `find_package(aws-c-common)` before the various module `include`s
Using `configure_package_config_file` makes it easier to have a relocatable package.
@yasminetalby yasminetalby added the needs-triage This issue or PR still needs to be triaged. label Sep 20, 2023
@jmklix
Copy link
Member

jmklix commented Nov 15, 2023

Can you make PR with the suggested changes?

@jmklix jmklix added bug We can reproduce the issue and confirmed it is a bug. p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Nov 15, 2023
@glaubitz
Copy link
Author

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.

@jmklix jmklix added the needs-review This issue or pull request needs review from a core team member. label Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug We can reproduce the issue and confirmed it is a bug. needs-review This issue or pull request needs review from a core team member. p2 This is a standard priority issue
Projects
None yet
5 participants